Re: [PATCH v2 net-next] tcp: md5: use kmalloc() backed scratch areas

2016-07-01 Thread David Miller
From: Eric Dumazet Date: Mon, 27 Jun 2016 18:51:53 +0200 > From: Eric Dumazet > > Some arches have virtually mapped kernel stacks, or will soon have. > > tcp_md5_hash_header() uses an automatic variable to copy tcp header > before mangling

Re: [PATCH v2 net-next] tcp: md5: use kmalloc() backed scratch areas

2016-06-29 Thread Herbert Xu
On Wed, Jun 29, 2016 at 03:39:37PM -0700, Andy Lutomirski wrote: > > I don't care about TCP MD5 performance at all. Ease of maintenance is > nice, though, and maybe there are other places in the kernel where > performance does matter. TCP MD5 is using ahash because it touches SG lists. Touching

Re: [PATCH v2 net-next] tcp: md5: use kmalloc() backed scratch areas

2016-06-29 Thread Andy Lutomirski
On Wed, Jun 29, 2016 at 2:44 PM, Eric Dumazet wrote: > On Wed, 2016-06-29 at 09:41 -0700, Andy Lutomirski wrote: > >> Overall, it looks like there's overhead of something like 50ns for >> each ahash invocation vs the shash equivalent. It's not huge, but >> it's there.

Re: [PATCH v2 net-next] tcp: md5: use kmalloc() backed scratch areas

2016-06-29 Thread Eric Dumazet
On Wed, 2016-06-29 at 09:41 -0700, Andy Lutomirski wrote: > Overall, it looks like there's overhead of something like 50ns for > each ahash invocation vs the shash equivalent. It's not huge, but > it's there. (This is cache-hot. I bet it's considerably worse if > cache-cold, because ahash will

Re: [PATCH v2 net-next] tcp: md5: use kmalloc() backed scratch areas

2016-06-29 Thread Andy Lutomirski
On Wed, Jun 29, 2016 at 8:38 AM, Herbert Xu wrote: > On Wed, Jun 29, 2016 at 08:26:43AM -0700, Andy Lutomirski wrote: >> >> Two reasons: >> >> 1. Code like tcp md5 would be simpler if it could pass a scatterlist >> to hash the skb but use a virtual address for the

Re: [PATCH v2 net-next] tcp: md5: use kmalloc() backed scratch areas

2016-06-29 Thread Herbert Xu
On Wed, Jun 29, 2016 at 08:26:43AM -0700, Andy Lutomirski wrote: > > Two reasons: > > 1. Code like tcp md5 would be simpler if it could pass a scatterlist > to hash the skb but use a virtual address for the header. True. But I bet we can make it simpler in other ways without creating special

Re: [PATCH v2 net-next] tcp: md5: use kmalloc() backed scratch areas

2016-06-29 Thread Andy Lutomirski
On Wed, Jun 29, 2016 at 8:02 AM, Herbert Xu wrote: > On Wed, Jun 29, 2016 at 07:59:22AM -0700, Andy Lutomirski wrote: >>> I suspect that, if you compare a synchronous implementation that can >> use virtual addresses to a DMA based implementation that can't, you'll >>

Re: [PATCH v2 net-next] tcp: md5: use kmalloc() backed scratch areas

2016-06-29 Thread Herbert Xu
On Wed, Jun 29, 2016 at 07:59:22AM -0700, Andy Lutomirski wrote: >> I suspect that, if you compare a synchronous implementation that can > use virtual addresses to a DMA based implementation that can't, you'll > find that, for short messages like tcp md5 uses, the synchronous > implementation

Re: [PATCH v2 net-next] tcp: md5: use kmalloc() backed scratch areas

2016-06-29 Thread Andy Lutomirski
On Tue, Jun 28, 2016 at 7:23 PM, Herbert Xu wrote: > On Tue, Jun 28, 2016 at 10:35:31AM -0700, Andy Lutomirski wrote: >> >> Do you mean this code: > > Yes. > >> I'm wondering why support for scatterlists is all-or-nothing. Why >> can't we initialize a hash object and

Re: [PATCH v2 net-next] tcp: md5: use kmalloc() backed scratch areas

2016-06-28 Thread Herbert Xu
On Tue, Jun 28, 2016 at 10:35:31AM -0700, Andy Lutomirski wrote: > > Do you mean this code: Yes. > I'm wondering why support for scatterlists is all-or-nothing. Why > can't we initialize a hash object and then alternate between passing > it scatterlists and pointers? Because once you have

Re: [PATCH v2 net-next] tcp: md5: use kmalloc() backed scratch areas

2016-06-28 Thread Andy Lutomirski
On Mon, Jun 27, 2016 at 8:41 PM, Herbert Xu wrote: > On Mon, Jun 27, 2016 at 10:58:42AM -0700, Andy Lutomirski wrote: >> >> I wonder if it's worth switching from ahash to shash, though. It >> would probably be simpler and faster. > > No shash is not appropriate here

Re: [PATCH v2 net-next] tcp: md5: use kmalloc() backed scratch areas

2016-06-27 Thread Herbert Xu
On Mon, Jun 27, 2016 at 10:58:42AM -0700, Andy Lutomirski wrote: > > I wonder if it's worth switching from ahash to shash, though. It > would probably be simpler and faster. No shash is not appropriate here because it needs to hash skb frags which are SG lists. Cheers, -- Email: Herbert Xu

Re: [PATCH v2 net-next] tcp: md5: use kmalloc() backed scratch areas

2016-06-27 Thread Eric Dumazet
On Mon, 2016-06-27 at 11:31 -0700, Cong Wang wrote: > Not a problem of your patch, but it seems these allocations never > get freed once we start using tcp md5. Maybe we should free them > when the last socket using tcp md5 is gone? If we constantly allocate-deallocate these tiny blocks for

Re: [PATCH v2 net-next] tcp: md5: use kmalloc() backed scratch areas

2016-06-27 Thread Eric Dumazet
On Mon, 2016-06-27 at 10:58 -0700, Andy Lutomirski wrote: > Seems reasonable. > > I wonder if it's worth switching from ahash to shash, though. It > would probably be simpler and faster. Well, I have no opinion on this, I will let a crypto guy doing this change if he cares ;) Thanks.

Re: [PATCH v2 net-next] tcp: md5: use kmalloc() backed scratch areas

2016-06-27 Thread Cong Wang
On Mon, Jun 27, 2016 at 9:51 AM, Eric Dumazet wrote: > diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c > index > 5c7ed147449c1b7ba029b12e033ad779a631460a..fddc0ab76c1df82cb05dba03271b773e3b2d > 100644 > --- a/net/ipv4/tcp.c > +++ b/net/ipv4/tcp.c > @@ -2969,8 +2969,18

Re: [PATCH v2 net-next] tcp: md5: use kmalloc() backed scratch areas

2016-06-27 Thread Andy Lutomirski
On Mon, Jun 27, 2016 at 9:51 AM, Eric Dumazet wrote: > From: Eric Dumazet > > Some arches have virtually mapped kernel stacks, or will soon have. > > tcp_md5_hash_header() uses an automatic variable to copy tcp header > before mangling th->check and

[PATCH v2 net-next] tcp: md5: use kmalloc() backed scratch areas

2016-06-27 Thread Eric Dumazet
From: Eric Dumazet Some arches have virtually mapped kernel stacks, or will soon have. tcp_md5_hash_header() uses an automatic variable to copy tcp header before mangling th->check and calling crypto function, which might be problematic on such arches. David says that