Re: [PATCH v3 net-next 1/4] tcp: ULP infrastructure

[Tom Herbert]

On Mon, Jul 31, 2017 at 3:16 PM, Dave Watson <davejwat...@fb.com> wrote:
> On 07/29/17 01:12 PM, Tom Herbert wrote:
>> On Wed, Jun 14, 2017 at 11:37 AM, Dave Watson <davejwat...@fb.com> wrote:
>> > Add the infrustructure for attaching Upper Layer Protocols (ULPs) over TCP
>> > sockets. Based on a similar infrastructure in tcp_cong.  The idea is that 
>> > any
>> > ULP can add its own logic by changing the TCP proto_ops structure to its 
>> > own
>> > methods.
>> >
>> > Example usage:
>> >
>> > setsockopt(sock, SOL_TCP, TCP_ULP, "tls", sizeof("tls"));
>> >
>> One question: is there a good reason why the ULP infrastructure should
>> just be for TCP sockets. For example, I'd really like to be able
>> something like:
>>
>> setsockopt(sock, SOL_SOCKET, SO_ULP, _param, sizeof(ulp_param));
>>
>> Where ulp_param is a structure containing the ULP name as well as some
>> ULP specific parameters that are passed to init_ulp. ulp_init could
>> determine whether the socket family is appropriate for the ULP being
>> requested.
>
> Using SOL_SOCKET instead seems reasonable to me.  I can see how
> ulp_params could have some use, perhaps at a slight loss in clarity.
> TLS needs its own setsockopts anyway though, for renegotiate for
> example.

I'll post the changes shortly. The reason to include parameters with
the setsockopt is so that we can push the ULP and start operations in
one shot.

Tom

Re: [PATCH v3 net-next 1/4] tcp: ULP infrastructure

[Tom Herbert]

On Fri, Jun 16, 2017 at 5:14 PM, Christoph Paasch
 wrote:
> Hello,
>
> On 14/06/17 - 11:37:14, Dave Watson wrote:
>> Add the infrustructure for attaching Upper Layer Protocols (ULPs) over TCP
>> sockets. Based on a similar infrastructure in tcp_cong.  The idea is that any
>> ULP can add its own logic by changing the TCP proto_ops structure to its own
>> methods.
>>
>> Example usage:
>>
>> setsockopt(sock, SOL_TCP, TCP_ULP, "tls", sizeof("tls"));
>>
>> modules will call:
>> tcp_register_ulp(_tls_ulp_ops);
>>
>> to register/unregister their ulp, with an init function and name.
>>
>> A list of registered ulps will be returned by tcp_get_available_ulp, which is
>> hooked up to /proc.  Example:
>>
>> $ cat /proc/sys/net/ipv4/tcp_available_ulp
>> tls
>>
>> There is currently no functionality to remove or chain ULPs, but
>> it should be possible to add these in the future if needed.
>>
>> Signed-off-by: Boris Pismenny 
>> Signed-off-by: Dave Watson 
>> ---
>>  include/net/inet_connection_sock.h |   4 ++
>>  include/net/tcp.h  |  25 +++
>>  include/uapi/linux/tcp.h   |   1 +
>>  net/ipv4/Makefile  |   2 +-
>>  net/ipv4/sysctl_net_ipv4.c |  25 +++
>>  net/ipv4/tcp.c |  28 
>>  net/ipv4/tcp_ipv4.c|   2 +
>>  net/ipv4/tcp_ulp.c | 134 
>> +
>>  8 files changed, 220 insertions(+), 1 deletion(-)
>>  create mode 100644 net/ipv4/tcp_ulp.c
>
> I know I'm pretty late to the game (and maybe this has already been
> discussed but I couldn't find anything in the archives), but I am wondering
> what the take is on potential races of the setsockopt() vs other system-calls.
>
> For example one might race the setsockopt() with a sendmsg() and the sendmsg
> might end up blocking on the lock_sock in tcp_sendmsg, waiting for
> tcp_set_ulp() to finish changing sk_prot. When the setsockopt() finishes, we
> are then inside tcp_sendmsg() coming directly from sendmsg(), while we
> should have been in the ULP's sendmsg.
>
> It seems like TLS-ULP is resilient to this (or at least, won't cause a panic),
> but there might be more exotic users of ULP in the future, that change other
> callbacks and then things might go wrong.

Christoph,

I noticed this also. I think the easiest answer would be to just
assume the caller understands the race condition and can synchronize
itself. Other than that we'd probably have wake up everyone blocking
on the socket without something like EAGAIN so they're forced to retry
the operation. But even that might not easily yield an obvious point
at which the socket can be safely changed.

Tom

>
>
> Thoughts?
>
>
> Thanks,
> Christoph
>

Re: [PATCH v3 net-next 1/4] tcp: ULP infrastructure

[Tom Herbert]

On Wed, Jun 14, 2017 at 11:37 AM, Dave Watson  wrote:
> Add the infrustructure for attaching Upper Layer Protocols (ULPs) over TCP
> sockets. Based on a similar infrastructure in tcp_cong.  The idea is that any
> ULP can add its own logic by changing the TCP proto_ops structure to its own
> methods.
>
> Example usage:
>
> setsockopt(sock, SOL_TCP, TCP_ULP, "tls", sizeof("tls"));
>
Hi Dave,

Thanks for this work. I think this functionality is going to have many uses!

One question: is there a good reason why the ULP infrastructure should
just be for TCP sockets. For example, I'd really like to be able
something like:

setsockopt(sock, SOL_SOCKET, SO_ULP, _param, sizeof(ulp_param));

Where ulp_param is a structure containing the ULP name as well as some
ULP specific parameters that are passed to init_ulp. ulp_init could
determine whether the socket family is appropriate for the ULP being
requested.

Thanks,
Tom

Re: [PATCH v3 net-next 0/4] kernel TLS

[Tom Herbert]

On Wed, Jun 14, 2017 at 3:17 PM, Dave Watson <davejwat...@fb.com> wrote:
> On 06/14/17 01:54 PM, Tom Herbert wrote:
>> On Wed, Jun 14, 2017 at 11:36 AM, Dave Watson <davejwat...@fb.com> wrote:
>> > This series adds support for kernel TLS encryption over TCP sockets.
>> > A standard TCP socket is converted to a TLS socket using a setsockopt.
>> > Only symmetric crypto is done in the kernel, as well as TLS record
>> > framing.  The handshake remains in userspace, and the negotiated
>> > cipher keys/iv are provided to the TCP socket.
>> >
>> I don't see support for TLS receive path in the kernel, only the send
>> path. Am I missing something?
>
> Correct, this is only TX.  Since it sounds likely some hardware might
> only be able to offload TX, we decided to configure TX and RX
> separately.  Using the OpenSSL patches, it should be transparent to
> users even if only one side is offloaded.
>
> The software RX patches exist but haven't been polished up yet.

Thanks for the clarification, looking forward to RX patches also!

Tom

Re: [PATCH v3 net-next 0/4] kernel TLS

[Tom Herbert]

On Wed, Jun 14, 2017 at 11:36 AM, Dave Watson  wrote:
> This series adds support for kernel TLS encryption over TCP sockets.
> A standard TCP socket is converted to a TLS socket using a setsockopt.
> Only symmetric crypto is done in the kernel, as well as TLS record
> framing.  The handshake remains in userspace, and the negotiated
> cipher keys/iv are provided to the TCP socket.
>
I don't see support for TLS receive path in the kernel, only the send
path. Am I missing something?

Thanks,
Tom

> We implemented support for this API in OpenSSL 1.1.0, the code is
> available at https://github.com/Mellanox/tls-openssl/tree/master
>
> It should work with any TLS library with similar modifications,
> a test tool using gnutls is here: https://github.com/Mellanox/tls-af_ktls_tool
>
> RFC patch to openssl:
> https://mta.openssl.org/pipermail/openssl-dev/2017-June/009384.html
>
> Changes from V2:
>
> * EXPORT_SYMBOL_GPL in patch 1
> * Ensure cleanup code always called before sk_stream_kill_queues to
>   avoid warnings
>
> Changes from V1:
>
> * EXPORT_SYMBOL GPL in patch 2
> * Add link to OpenSSL patch & gnutls example in documentation patch.
> * sk_write_pending check was rolled in to wait_for_memory path,
>   avoids special case and fixes lock inbalance issue.
> * Unify flag handling for sendmsg/sendfile
>
> Changes from RFC V2:
>
> * Generic ULP (upper layer protocol) framework instead of TLS specific
>   setsockopts
> * Dropped Mellanox hardware patches, will come as separate series.
>   Framework will work for both.
>
> RFC V2:
>
> http://www.mail-archive.com/netdev@vger.kernel.org/msg160317.html
>
> Changes from RFC V1:
>
> * Socket based on changing TCP proto_ops instead of crypto framework
> * Merged code with Mellanox's hardware tls offload
> * Zerocopy sendmsg support added - sendpage/sendfile is no longer
>   necessary for zerocopy optimization
>
> RFC V1:
>
> http://www.mail-archive.com/netdev@vger.kernel.org/msg88021.html
>
> * Socket based on crypto userspace API framework, required two
>   sockets in userspace, one encrypted, one unencrypted.
>
> Paper: https://netdevconf.org/1.2/papers/ktls.pdf
>
> Aviad Yehezkel (1):
>   tcp: export do_tcp_sendpages and tcp_rate_check_app_limited functions
>
> Boris Pismenny (2):
>   tcp: ULP infrastructure
>   tls: Documentation
>
> Ilya Lesokhin (1):
>   tls: kernel TLS support
>
>  Documentation/networking/tls.txt   | 135 +++
>  MAINTAINERS|  10 +
>  include/linux/socket.h |   1 +
>  include/net/inet_connection_sock.h |   4 +
>  include/net/tcp.h  |  27 ++
>  include/net/tls.h  | 237 
>  include/uapi/linux/tcp.h   |   1 +
>  include/uapi/linux/tls.h   |  79 
>  net/Kconfig|   1 +
>  net/Makefile   |   1 +
>  net/ipv4/Makefile  |   2 +-
>  net/ipv4/sysctl_net_ipv4.c |  25 ++
>  net/ipv4/tcp.c |  33 +-
>  net/ipv4/tcp_ipv4.c|   2 +
>  net/ipv4/tcp_rate.c|   1 +
>  net/ipv4/tcp_ulp.c | 134 +++
>  net/tls/Kconfig|  12 +
>  net/tls/Makefile   |   7 +
>  net/tls/tls_main.c | 487 +++
>  net/tls/tls_sw.c   | 772 
> +
>  20 files changed, 1968 insertions(+), 3 deletions(-)
>  create mode 100644 Documentation/networking/tls.txt
>  create mode 100644 include/net/tls.h
>  create mode 100644 include/uapi/linux/tls.h
>  create mode 100644 net/ipv4/tcp_ulp.c
>  create mode 100644 net/tls/Kconfig
>  create mode 100644 net/tls/Makefile
>  create mode 100644 net/tls/tls_main.c
>  create mode 100644 net/tls/tls_sw.c
>
> --
> 2.9.3
>

Re: [RFC TLS Offload Support 05/15] tcp: Add TLS socket options for TCP sockets

[Tom Herbert]

On Tue, Mar 28, 2017 at 6:26 AM, Aviad Yehezkel  wrote:
> This patch adds TLS_TX and TLS_RX TCP socket options.
>
> Setting these socket options will change the sk->sk_prot
> operations of the TCP socket. The user is responsible to
> prevent races between calls to the previous operations
> and the new operations. After successful return, data
> sent on this socket will be encapsulated in TLS.
>
> Signed-off-by: Aviad Yehezkel 
> Signed-off-by: Boris Pismenny 
> Signed-off-by: Ilya Lesokhin 
> ---
>  include/uapi/linux/tcp.h |  2 ++
>  net/ipv4/tcp.c   | 32 
>  2 files changed, 34 insertions(+)
>
> diff --git a/include/uapi/linux/tcp.h b/include/uapi/linux/tcp.h
> index c53de26..f9f0e29 100644
> --- a/include/uapi/linux/tcp.h
> +++ b/include/uapi/linux/tcp.h
> @@ -116,6 +116,8 @@ enum {
>  #define TCP_SAVE_SYN   27  /* Record SYN headers for new 
> connections */
>  #define TCP_SAVED_SYN  28  /* Get SYN headers recorded for 
> connection */
>  #define TCP_REPAIR_WINDOW  29  /* Get/set window parameters */
> +#define TCP_TLS_TX 30
> +#define TCP_TLS_RX 31
>
>  struct tcp_repair_opt {
> __u32   opt_code;
> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> index 302fee9..2d190e3 100644
> --- a/net/ipv4/tcp.c
> +++ b/net/ipv4/tcp.c
> @@ -273,6 +273,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -2676,6 +2677,21 @@ static int do_tcp_setsockopt(struct sock *sk, int 
> level,
> tp->notsent_lowat = val;
> sk->sk_write_space(sk);
> break;
> +   case TCP_TLS_TX:
> +   case TCP_TLS_RX: {
> +   int (*fn)(struct sock *sk, int optname,
> + char __user *optval, unsigned int optlen);
> +
> +   fn = symbol_get(tls_sk_attach);
> +   if (!fn) {
> +   err = -EINVAL;
> +   break;
> +   }
> +
> +   err = fn(sk, optname, optval, optlen);
> +   symbol_put(tls_sk_attach);
> +   break;
> +   }
> default:
> err = -ENOPROTOOPT;
> break;
> @@ -3064,6 +3080,22 @@ static int do_tcp_getsockopt(struct sock *sk, int 
> level,
> }
> return 0;
> }
> +   case TCP_TLS_TX:
> +   case TCP_TLS_RX: {
> +   int err;
> +   int (*fn)(struct sock *sk, int optname,
> + char __user *optval, int __user *optlen);
> +
> +   fn = symbol_get(tls_sk_query);
> +   if (!fn) {
> +   err = -EINVAL;
> +   break;
> +   }
> +
> +   err = fn(sk, optname, optval, optlen);
> +   symbol_put(tls_sk_query);
> +   return err;
> +   }

This mechanism should be generalized. If we can do this with TLS then
there will likely be other ULPs that we might want to set on a TCP
socket. Maybe something like TCP_ULP_PUSH, TCP_ULP_POP (borrowing from
STREAMS ever so slightly :-) ). I'd also suggest that the ULPs are
indicated by a text string in the socket option argument, then have
each ULP perform a registration for their service.


> default:
> return -ENOPROTOOPT;
> }
> --
> 2.7.4
>

Re: [PATCH v5 1/4] siphash: add cryptographically secure PRF

[Tom Herbert]

On Fri, Dec 16, 2016 at 12:41 PM, George Spelvin
<li...@sciencehorizons.net> wrote:
> Tom Herbert wrote:
>> Tested this. Distribution and avalanche effect are still good. Speed
>> wise I see about a 33% improvement over siphash (20 nsecs/op versus 32
>> nsecs). That's about 3x of jhash speed (7 nsecs). So that might closer
>> to a more palatable replacement for jhash. Do we lose any security
>> advantages with halfsiphash?
>
> What are you testing on?  And what input size?  And does "33% improvement"
> mean 4/3 the rate and 3/4 the time?  Or 2/3 the time and 3/2 the rate?
>
Sorry, that is over an IPv4 tuple. Intel(R) Xeon(R) CPU E5-2660 0 @
2.20GHz. Recoded the function I was using to look like more like 64
bit version and yes it is indeed slower.

> These are very odd results.  On a 64-bit machine, SipHash should be the
> same speed per round, and faster because it hashes more data per round.
> (Unless you're hitting some unexpected cache/decode effect due to REX
> prefixes.)
>
> On a 32-bit machine (other than ARM, where your results might make sense,
> or maybe if you're hashing large amounts of data), the difference should
> be larger.
>
> And yes, there is a *significant* security loss.  SipHash is 128 bits
> ("don't worry about it").  hsiphash is 64 bits, which is known breakable
> ("worry about it"), so we have to do a careful analysis of the cost of
> a successful attack.
>
> As mentioned in the e-mails that just flew by, hsiphash is intended
> *only* for 32-bit machines which bog down on full SipHash.  On all 64-bit
> machines, it will be implemented as an alias for SipHash and the security
> concerns will Just Go Away.
>
> The place where hsiphash is expected to make a big difference is 32-bit
> x86.  If you only see 33% difference with "gcc -m32", I'm going to be
> very confused.
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 1/4] siphash: add cryptographically secure PRF

[Tom Herbert]

On Fri, Dec 16, 2016 at 4:39 AM, Jason A. Donenfeld  wrote:
> Hey JP,
>
> On Fri, Dec 16, 2016 at 9:08 AM, Jean-Philippe Aumasson
>  wrote:
>> Here's a tentative HalfSipHash:
>> https://github.com/veorq/SipHash/blob/halfsiphash/halfsiphash.c
>>
>> Haven't computed the cycle count nor measured its speed.
>
Tested this. Distribution and avalanche effect are still good. Speed
wise I see about a 33% improvement over siphash (20 nsecs/op versus 32
nsecs). That's about 3x of jhash speed (7 nsecs). So that might closer
to a more palatable replacement for jhash. Do we lose any security
advantages with halfsiphash?

Tom

> This is incredible. Really. Wow!
>
> I'll integrate this into my patchset and will write up some
> documentation about when one should be used over the other.
>
> Thanks again. Quite exciting.
>
> Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 1/3] siphash: add cryptographically secure hashtable function

[Tom Herbert]

On Wed, Dec 14, 2016 at 2:56 PM, Jason A. Donenfeld <ja...@zx2c4.com> wrote:
> Hey Tom,
>
> On Wed, Dec 14, 2016 at 10:35 PM, Tom Herbert <t...@herbertland.com> wrote:
>> Those look good, although I would probably just do 1,2,3 words and
>> then have a function that takes n words like jhash. Might want to call
>> these dword to distinguish from 32 bit words in jhash.
>
> So actually jhash_Nwords makes no sense, since it takes dwords
> (32-bits) not words (16-bits). The siphash analog should be called
> siphash24_Nqwords.
>
Yeah, that's a "bug" with jhash function names.

> I think what I'll do is change what I already have to:
> siphash24_1qword
> siphash24_2qword
> siphash24_3qword
> siphash24_4qword
>
> And then add some static inline helpers to assist with smaller u32s
> like ipv4 addresses called:
>
> siphash24_2dword
> siphash24_4dword
> siphash24_6dword
> siphash24_8dword
>
> While we're having something new, might as well call it the right thing.
>
I'm confused, doesn't 2dword == 1qword? Anyway, I think the qword
functions are good enough. If someone needs to hash over some odd
length they can either put them in a structure padded to 64 bits or
call the hash function that takes a byte length.

>
>> Also, what is the significance of "24" in the function and constant
>> names? Can we just drop that and call this siphash?
>
> SipHash is actually a family of PRFs, differentiated by the number of
> SIPROUNDs after each 64-bit input is processed and the number of
> SIPROUNDs at the very end of the function. The best trade-off of speed
> and security for kernel usage is 2 rounds after each 64-bit input and
> 4 rounds at the end of the function. This doesn't fall to any known
> cryptanalysis and it's very fast.

I'd still drop the "24" unless you really think we're going to have
multiple variants coming into the kernel.

Tom
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 3/4] secure_seq: use siphash24 instead of md5_transform

[Tom Herbert]

On Wed, Dec 14, 2016 at 4:53 AM, Jason A. Donenfeld  wrote:
> Hi David,
>
> On Wed, Dec 14, 2016 at 10:51 AM, David Laight  
> wrote:
>> From: Jason A. Donenfeld
>>> Sent: 14 December 2016 00:17
>>> This gives a clear speed and security improvement. Rather than manually
>>> filling MD5 buffers, we simply create a layout by a simple anonymous
>>> struct, for which gcc generates rather efficient code.
>> ...
>>> + const struct {
>>> + struct in6_addr saddr;
>>> + struct in6_addr daddr;
>>> + __be16 sport;
>>> + __be16 dport;
>>> + } __packed combined = {
>>> + .saddr = *(struct in6_addr *)saddr,
>>> + .daddr = *(struct in6_addr *)daddr,
>>> + .sport = sport,
>>> + .dport = dport
>>> + };
>>
>> You need to look at the effect of marking this (and the other)
>> structures 'packed' on architectures like sparc64.
>
> In all current uses of __packed in the code, I think the impact is
> precisely zero, because all structures have members in descending
> order of size, with each member being a perfect multiple of the one
> below it. The __packed is therefore just there for safety, in case
> somebody comes in and screws everything up by sticking a u8 in
> between. In that case, it wouldn't be desirable to hash the structure
> padding bits. In the worst case, I don't believe the impact would be
> worse than a byte-by-byte memcpy, which is what the old code did. But
> anyway, these structures are already naturally packed anyway, so the
> present impact is nil.
>
If you pad the data structure to 64 bits then we can call the version
of siphash that only deals in 64 bit words. Writing a zero in the
padding will be cheaper than dealing with odd lengths in siphash24.

Tom

> Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 1/3] siphash: add cryptographically secure hashtable function

[Tom Herbert]

On Wed, Dec 14, 2016 at 10:46 AM, Jason A. Donenfeld  wrote:
> SipHash is a 64-bit keyed hash function that is actually a
> cryptographically secure PRF, like HMAC. Except SipHash is super fast,
> and is meant to be used as a hashtable keyed lookup function.
>
"super fast" is relative. My quick test shows that this faster than
Toeplitz (good, but not exactly hard to achieve), but is about 4x
slower than jhash.

> SipHash isn't just some new trendy hash function. It's been around for a
> while, and there really isn't anything that comes remotely close to
> being useful in the way SipHash is. With that said, why do we need this?
>
I don't think we need advertising nor a lesson on hashing. It would be
much more useful if you just point us to the paper on siphash (which I
assume I http://cr.yp.to/siphash/siphash-20120918.pdf ?).

> There are a variety of attacks known as "hashtable poisoning" in which an
> attacker forms some data such that the hash of that data will be the
> same, and then preceeds to fill up all entries of a hashbucket. This is
> a realistic and well-known denial-of-service vector.
>
> Linux developers already seem to be aware that this is an issue, and
> various places that use hash tables in, say, a network context, use a
> non-cryptographically secure function (usually jhash) and then try to
> twiddle with the key on a time basis (or in many cases just do nothing
> and hope that nobody notices). While this is an admirable attempt at
> solving the problem, it doesn't actually fix it. SipHash fixes it.
>
Key rotation is important anyway, without any key rotation even if the
key is compromised in siphash by some external means we would have an
insecure hash until the system reboots.

> (It fixes it in such a sound way that you could even build a stream
> cipher out of SipHash that would resist the modern cryptanalysis.)
>
> There are a modicum of places in the kernel that are vulnerable to
> hashtable poisoning attacks, either via userspace vectors or network
> vectors, and there's not a reliable mechanism inside the kernel at the
> moment to fix it. The first step toward fixing these issues is actually
> getting a secure primitive into the kernel for developers to use. Then
> we can, bit by bit, port things over to it as deemed appropriate.
>
> Secondly, a few places are using MD5 for creating secure sequence
> numbers, port numbers, or fast random numbers. Siphash is a faster, more
> fittting, and more secure replacement for MD5 in those situations.
>
> Dozens of languages are already using this internally for their hash
> tables. Some of the BSDs already use this in their kernels. SipHash is
> a widely known high-speed solution to a widely known problem, and it's
> time we catch-up.
>
Maybe so, but we need to do due diligence before considering adopting
siphash as the primary hashing in the network stack. Consider that we
may very well perform a hash over L4 tuples on _every_ packet. We've
done a good job at limiting this to be at most one hash per packet,
but nevertheless the performance of the hash function must be take
into account.

A few points:

1) My quick test shows siphash is about four times more expensive than
jhash. On my test system, computing a hash over IPv4 tuple (two 32 bit
addresses and 2 16 bit source ports) is 6.9 nsecs in Jenkins hash, 33
nsecs with siphash. Given that we have eliminated most of the packet
header hashes this might be tolerable, but still should be looking at
ways to optimize.
2) I like moving to use u64 (quad words) in the hash, this is an
improvement over Jenkins which is based on 32 bit words. If we put
this in the kernel we probably want to have several variants of
siphash for specific sizes (e.g. siphash1, siphash2, siphash2,
siphashn for hash over one, two, three, or n sixty four bit words).
3) I also tested siphash distribution and Avalanche Effect (these
really should have been covered in the paper :-( ). Both of these are
good with siphash, in fact almost have identical characteristics to
Jenkins hash.

Tom

> Signed-off-by: Jason A. Donenfeld 
> Cc: Jean-Philippe Aumasson 
> Cc: Daniel J. Bernstein 
> Cc: Linus Torvalds 
> Cc: Eric Biggers 
> Cc: David Laight 
> ---
> Changes from v2->v3:
>
>   - There is now a fast aligned version of the function and a not-as-fast
> unaligned version. The requirements for each have been documented in
> a docbook-style comment. As well, the header now contains a constant
> for the expected alignment.
>
>   - The test suite has been updated to check both the unaligned and aligned
> version of the function.
>
>  include/linux/siphash.h |  30 ++
>  lib/Kconfig.debug   |   6 +-
>  lib/Makefile|   5 +-
>  lib/siphash.c   | 153 
> 
>  lib/test_siphash.c  |  85 

Re: [PATCH 0/3] crypto: af_alg - add TLS type encryption

[Tom Herbert]

On Thu, Apr 7, 2016 at 11:52 PM, Herbert Xu  wrote:
> On Wed, Apr 06, 2016 at 10:56:12AM -0700, Tadeusz Struk wrote:
>>
>> The intend is to enable HW acceleration of the TLS protocol.
>> The way it will work is that the user space will send a packet of data
>> via AF_ALG and HW will authenticate and encrypt it in one go.
>
> There have been suggestions to implement TLS data-path within
> the kernel.  So we should decide whether we pursue that or go
> with your approach before we start adding algorithms.
>
Yes, please see Dave Watson's patches on this.

Tom

> Cheers,
> --
> Email: Herbert Xu 
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: ipsec impact on performance

[Tom Herbert]

On Wed, Dec 2, 2015 at 1:12 PM, Sowmini Varadhan
<sowmini.varad...@oracle.com> wrote:
> On (12/02/15 13:07), Tom Herbert wrote:
>> That's easy enough to add to flow dissector, but is SPI really
>> intended to be used an L4 entropy value? We would need to consider the
>
> yes. To quote https://en.wikipedia.org/wiki/Security_Parameter_Index
> "This works like port numbers in TCP and UDP connections. What it means
>  is that there could be different SAs used to provide security to one
>  connection. An SA could therefore act as a set of rules."
>
>> effects of running multiple TCP connections over an IPsec. Also, you
>> might want to try IPv6, the flow label should provide a good L4 hash
>> for RPS/RFS, it would be interesting to see what the effects are with
>> IPsec processing. (ESP/UDP could also if RSS/ECMP is critical)
>
> IPv6 would be an interesting academic exercise, but it's going
> to be a while before we get RDS-TCP to go over IPv6.
>
Huh? Who said anything about RDS-TCP? I thought you were trying to
improve IPsec performance...
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: ipsec impact on performance

[Tom Herbert]

On Wed, Dec 2, 2015 at 12:50 PM, Sowmini Varadhan
 wrote:
> On (12/02/15 12:41), David Laight wrote:
>> You are getting 0.7 Gbps with ass-ccm-a-128, scale the esp-null back to
>> that and it would use 7/18*71 = 27% of the cpu.
>> So 69% of the cpu in the a-128 case is probably caused by the
>> encryption itself.
>> Even if the rest of the code cost nothing you'd not increase
>> above 1Gbps.
>
> Fortunately, the situation is not quite hopeless yet.
>
> Thanks to Rick Jones for supplying the hints for this, but with
> some careful manual pinning of irqs and iperf processes to cpus,
> I can get to 4.5 Gbps for the esp-null case.
>
> Given that the [clear traffic + GSO without GRO] gets me about 5-7 Gbps,
> the 4.5 Gbps is not that far off (and at that point, the nickel-and-dime
> tweaks may help even more).
>
> For AES-GCM, I'm able to go from 1.8 Gbps (no GSO) to 2.8 Gbps.
> Still not great, but proves that we haven't yet hit any upper bounds
> yet.
>
> I think a lot of the manual tweaking of irq/process placement
> is needed because the existing rps/rfs flow steering is looking
> for TCP/UDP flow numbers to do the steering. It can just as easily
> use the IPsec SPI numbers to do this, and that's another place where
> we can make this more ipsec-friendly.
>
That's easy enough to add to flow dissector, but is SPI really
intended to be used an L4 entropy value? We would need to consider the
effects of running multiple TCP connections over an IPsec. Also, you
might want to try IPv6, the flow label should provide a good L4 hash
for RPS/RFS, it would be interesting to see what the effects are with
IPsec processing. (ESP/UDP could also if RSS/ECMP is critical)

Tom
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: ipsec impact on performance

[Tom Herbert]

On Tue, Dec 1, 2015 at 9:59 AM, Sowmini Varadhan
 wrote:
>
> I instrumented iperf with and without ipsec, just using esp-null,
> and 1 thread, to keep things simple. I'm seeing some pretty dismal
> performance numbers with ipsec, and trying to think of ways to
> improve this. Here are my findings, please share feedback.
>
> I suspect that a big part of the problem is the implicit loss of GSO,
> and this is made worse by some inefficiencies in the xfrm code:
> for single stream iperf (to avoid effects of rx-hash), I see the
> following on a 10G p2p ethernet link.
>  8.5-9.5 Gbps clear traffic, TSO disabled, so GSO, GRO is in effect
>  3-4 Gbps clear traffic, with both TSO/GSO disabled
>  1.8-2 Gbps for esp-null.

Are you losing checksum offload also?

> So the above numbers suggest that losing TSO/GSO results in one
> big drop in performance, and then there's another cliff for the
> clear -> esp-null transition. And those cliffs apply even if you are
> merely doing TCP-MD5 or AO for basic protection of the TCP connection.
>
> I tried moving things about a bit to defer the ipsec after GSO - I'll
> share my experimental patch as an RFC in a separate thread. (Disclaimer:
> the patch is just an experiment at this point).
>
> In that patch, I'm only focussing on esp-null and transp-mode ipsec
> for now, just to get some basic performance numbers to see if this is
> at all interesting.  Essentially my hack mainly involves the following
>
> - don't disable TSO in sk_setup_caps() if a dst->header_len is found
> - in xfrm4_output, if GSO is applicable, bail out without esp header
>   addition - that will get done after skb_segment()
> - at the end of tcp_gso_segment() (when tcp segment is available),
>   set things up for xfrm_output_one and trigger the esp_output..
>   I have to be very careful about setting up skb pointers here, since
>   it looks like esp_output overloads the mac_header pointer e.g., for
>   setting up the ip protocol field
>
> If I do all these things, the ipsec+iperf improves slightly- for
> esp-null, I move from approx 1.8 Gbps  to about 3 Gbps, but clearly,
> this is still quite far from the 8 - 9 Gbps that I can get with just
> GSO+GRO for non-ipsec traffic.
>
> There are some inefficiencies that I can see in the xfrm code,
> that I am inheriting in my patch, e.g.,:
>   memory management in the xfrm code has room for improvement. Every
>   pass through xfrm_transport_output ends up doing a (avoidable?) memmove,
>   and each pass through esp_output ends up doing a kmalloc/free of the
>   "tmp" buffer.
> But these are all still relatively small things - tweaking them
> doesnt get me significantly past the 3 Gbps limit. Any suggestions
> on how to make this budge (or design criticism of the patch) would
> be welcome.
>
Thanks for the nice data! We could certainly implement GRO/GSO for
esp-null to get your numbers up but I don't think that would be very
useful to anyone. Do you have the performance numbers using real
encryption?

> --Sowmini
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html