Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933268AbcKGSXQ (ORCPT ); Mon, 7 Nov 2016 13:23:16 -0500 Received: from frisell.zx2c4.com ([192.95.5.64]:35465 "EHLO frisell.zx2c4.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932873AbcKGSXM (ORCPT ); Mon, 7 Nov 2016 13:23:12 -0500 MIME-Version: 1.0 In-Reply-To: References: <20161103004934.GA30775@gondor.apana.org.au> <20161103.130852.1456848512897088071.davem@davemloft.net> <20161104173723.GB34176@google.com> From: "Jason A. Donenfeld" Date: Mon, 7 Nov 2016 19:23:05 +0100 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH] poly1305: generic C can be faster on chips with slow unaligned access To: Eric Biggers Cc: David Miller , Herbert Xu , linux-crypto@vger.kernel.org, LKML , Martin Willi , WireGuard mailing list , =?UTF-8?Q?Ren=C3=A9_van_Dorst?= Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1614 Lines: 41 On Mon, Nov 7, 2016 at 7:08 PM, Jason A. Donenfeld wrote: > Hmm... The general data flow that strikes me as most pertinent is > something like: > > struct sk_buff *skb = get_it_from_somewhere(); > skb = skb_share_check(skb, GFP_ATOMIC); > num_frags = skb_cow_data(skb, ..., ...); > struct scatterlist sg[num_frags]; > sg_init_table(sg, num_frags); > skb_to_sgvec(skb, sg, ..., ...); > blkcipher_walk_init(&walk, sg, sg, len); > blkcipher_walk_virt_block(&desc, &walk, BLOCK_SIZE); > while (walk.nbytes >= BLOCK_SIZE) { > size_t chunk_len = rounddown(walk.nbytes, BLOCK_SIZE); > poly1305_update(&poly1305_state, walk.src.virt.addr, chunk_len); > blkcipher_walk_done(&desc, &walk, walk.nbytes % BLOCK_SIZE); > } > if (walk.nbytes) { > poly1305_update(&poly1305_state, walk.src.virt.addr, walk.nbytes); > blkcipher_walk_done(&desc, &walk, 0); > } > > Is your suggestion that that in the final if block, walk.src.virt.addr > might be unaligned? Like in the case of the last fragment being 67 > bytes long? In fact, I'm not so sure this happens here. In the while loop, each new walk.src.virt.addr will be aligned to BLOCK_SIZE or be aligned by virtue of being at the start of a new page. In the subsequent if block, walk.src.virt.addr will either be some_aligned_address+BLOCK_SIZE, which will be aligned, or it will be a start of a new page, which will be aligned. So what did you have in mind exactly? I don't think anybody is running code like: for (size_t i = 0; i < len; i += 17) poly1305_update(&poly, &buffer[i], 17); (And if so, those consumers should be fixed.)