* Re: [PATCH v6 0/4] Broadcom SBA RAID support
From: Vinod Koul @ 2017-03-30 4:49 UTC (permalink / raw)
To: Jassi Brar
Cc: Anup Patel, Rob Herring, Mark Rutland, Herbert Xu,
David S . Miller, Dan Williams, Ray Jui, Scott Branden, Jon Mason,
Rob Rice, BCM Kernel Feedback, dmaengine, Device Tree,
Linux ARM Kernel, Linux Kernel, linux-crypto, linux-raid
In-Reply-To: <CAALAos-P6CDdWTJxg8YQfT1V+iCxUcbYHiQ35EdZzz9PnN3cuA@mail.gmail.com>
On Wed, Mar 29, 2017 at 11:35:43AM +0530, Anup Patel wrote:
> On Tue, Mar 21, 2017 at 2:48 PM, Vinod Koul <vinod.koul@intel.com> wrote:
> > On Tue, Mar 21, 2017 at 02:17:21PM +0530, Anup Patel wrote:
> >> On Tue, Mar 21, 2017 at 2:00 PM, Vinod Koul <vinod.koul@intel.com> wrote:
> >> > On Mon, Mar 06, 2017 at 03:13:24PM +0530, Anup Patel wrote:
> >> >> The Broadcom SBA RAID is a stream-based device which provides
> >> >> RAID5/6 offload.
> >> >>
> >> >> It requires a SoC specific ring manager (such as Broadcom FlexRM
> >> >> ring manager) to provide ring-based programming interface. Due to
> >> >> this, the Broadcom SBA RAID driver (mailbox client) implements
> >> >> DMA device having one DMA channel using a set of mailbox channels
> >> >> provided by Broadcom SoC specific ring manager driver (mailbox
> >> >> controller).
> >> >>
> >> >> The Broadcom SBA RAID hardware requires PQ disk position instead
> >> >> of PQ disk coefficient. To address this, we have added raid_gflog
> >> >> table which will help driver to convert PQ disk coefficient to PQ
> >> >> disk position.
> >> >>
> >> >> This patchset is based on Linux-4.11-rc1 and depends on patchset
> >> >> "[PATCH v5 0/2] Broadcom FlexRM ring manager support"
> >> >
> >> > Okay I applied and was about to push when I noticed this :(
> >> >
> >> > So what is the status of this..?
> >>
> >> PATCH2 is Acked but PATCH1 is under-review. Currently, its
> >> v6 of that patchset.
> >>
> >> The only dependency on that patchset is the changes in
> >> brcm-message.h which are required by this BCM-SBA-RAID
> >> driver.
> >>
> >> @Jassi,
> >> Can you please have a look at PATCH v6?
> >
> > And I would need an immutable branch/tag once merged. I am going to keep
> > this series pending till then.
>
> The Broadcom FlexRM patchset is pickedup by Jassi and
> can be found in mailbox-for-next branch of
> git://git.linaro.org/landing-teams/working/fujitsu/integration
>
> Both patchset (Broadcom FlexRM patchset and this one) are
> also available in sba-raid-v7 branch of
> https://github.com/Broadcom/arm64-linux.git
Jassi,
Can you provide an immutable branch/tag please for this, latter is
preferred.
Btw didn't find your tree in MAINTAINERS..
>
> Regards,
> Anup
--
~Vinod
^ permalink raw reply
* Re: [PATCH 1/6] virtio: wrap find_vqs
From: Jason Wang @ 2017-03-30 6:00 UTC (permalink / raw)
To: Michael S. Tsirkin, linux-kernel
Cc: John Fastabend, Arnd Bergmann, Greg Kroah-Hartman, Amit Shah,
Gonglei, Herbert Xu, David S. Miller, David Airlie, Gerd Hoffmann,
Dmitry Tarnyagin, Ohad Ben-Cohen, Bjorn Andersson,
James E.J. Bottomley, Martin K. Petersen, Stefan Hajnoczi,
virtualization, linux-crypto, dri-devel, netdev, linux-remoteproc
In-Reply-To: <1490820507-8005-2-git-send-email-mst@redhat.com>
On 2017年03月30日 04:48, Michael S. Tsirkin wrote:
> We are going to add more parameters to find_vqs, let's wrap the call so
> we don't need to tweak all drivers every time.
>
> Signed-off-by: Michael S. Tsirkin<mst@redhat.com>
> ---
A quick glance and it looks ok, but what the benefit of this series, is
it required by other changes?
Thanks
^ permalink raw reply
* RE: [RFC TLS Offload Support 00/15] cover letter
From: Boris Pismenny @ 2017-03-30 6:49 UTC (permalink / raw)
To: Hannes Frederic Sowa, David Miller, Aviad Yehezkel
Cc: Ilya Lesokhin, davejwatson@fb.com, netdev@vger.kernel.org,
Matan Barak, Liran Liss, Haggai Eran, tom@herbertland.com,
herbert@gondor.apana.org.au, nmav@gnults.org,
fridolin.pokorny@gmail.com, Ilan Tayari, Yevgeny Kliteynik,
linux-crypto@vger.kernel.org, Saeed Mahameed,
aviadye@dev.mellanox.co.il
In-Reply-To: <fae7c3d4-5143-6f66-9551-a26d7db7de51@stressinduktion.org>
> >> TLS Tx crypto offload is a new feature of network devices. It enables
> >> the kernel TLS socket to skip encryption and authentication
> >> operations on the transmit side of the data path, delegating those to
> >> the NIC. In turn, the NIC encrypts packets that belong to an
> >> offloaded TLS socket on the fly. The NIC does not modify any packet
> >> headers. It expects to receive fully framed TCP packets with TLS
> >> records as payload. The NIC replaces plaintext with ciphertext and
> >> fills the authentication tag. The NIC does not hold any state beyond
> >> the context needed to encrypt the next expected packet, i.e. expected
> >> TCP sequence number and crypto state.
> >
> > It seems like, since you do the TLS framing in TCP and the card is
> > expecting to fill in certain aspects, there is a requirement that the
> > packet contents aren't mangled between the TLS framing code and when
> > the SKB hits the card.
> >
> > Is this right?
> >
> > For example, what happens if netfilter splits a TLS Tx offloaded frame
> > into two TCP segments?
We maintain the crypto context by tracking TCP sequence numbers, splitting
TCP segments is not a problem. Even if reordering is introduced anywhere
between TCP and the driver, we can identify it according to the TCP
sequence number and handle it gracefully – see mlx_tls_tx_handler.
>
> Furthermore, it doesn't seem to work with bonding or any other virtual
> interface, which could move the skb's to be processed on another NIC, as the
> context is put onto the NIC. Even a redirect can not be processed anymore
> (seems like those patches try to stick the connection to an interface anyway).
>
> Wouldn't it be possible to keep the state in software and push down a
> security context per skb, which get applied during sending? If not possible via
> hw, slowpath can encrypt packet in sw.
We do have all the state required to encrypt a TLS packet in software. But,
pushing down the state for each skb is too expansive, because the state
depends on all data in the TLS record, essentially it requires to resend the
record up to that skb. This is accomplished for OOO packets in the
“handle_ooo” function in mlx_tls.
Maybe we could use that functionality to handle bonding, but at first it would
be easier to prevent it.
The slowpath you’ve mentioned is tricky, because you need to decide in
advance that a TLS record will use the slowpath, because after the plaintext
TLS record is pushed into TCP it is difficult to fallback to software crypto.
>
> Also sticking connections to outgoing interfaces might work for TX, but you
> can't force the interface where packets come in.
Right. This RFC handles only TX offload.
>
> Bye,
> Hannes
Thanks,
Boris
^ permalink raw reply
* Re: [PATCH 1/6] virtio: wrap find_vqs
From: Cornelia Huck @ 2017-03-30 7:18 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: linux-kernel, Dmitry Tarnyagin, kvm, David Airlie,
linux-remoteproc, dri-devel, Bjorn Andersson,
James E.J. Bottomley, Herbert Xu, linux-scsi, John Fastabend,
Arnd Bergmann, Amit Shah, Stefan Hajnoczi, virtualization,
Martin K. Petersen, Greg Kroah-Hartman, linux-crypto, netdev,
David S. Miller
In-Reply-To: <1490820507-8005-2-git-send-email-mst@redhat.com>
On Wed, 29 Mar 2017 23:48:44 +0300
"Michael S. Tsirkin" <mst@redhat.com> wrote:
> We are going to add more parameters to find_vqs, let's wrap the call so
> we don't need to tweak all drivers every time.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> drivers/block/virtio_blk.c | 3 +--
> drivers/char/virtio_console.c | 6 +++---
> drivers/crypto/virtio/virtio_crypto_core.c | 3 +--
> drivers/gpu/drm/virtio/virtgpu_kms.c | 3 +--
> drivers/net/caif/caif_virtio.c | 3 +--
> drivers/net/virtio_net.c | 3 +--
> drivers/rpmsg/virtio_rpmsg_bus.c | 2 +-
> drivers/scsi/virtio_scsi.c | 3 +--
> drivers/virtio/virtio_balloon.c | 3 +--
> drivers/virtio/virtio_input.c | 3 +--
> include/linux/virtio_config.h | 9 +++++++++
> net/vmw_vsock/virtio_transport.c | 6 +++---
> 12 files changed, 24 insertions(+), 23 deletions(-)
Regardless whether that context thing is the right thing to do, this
looks like a sensible cleanup.
^ permalink raw reply
* Re: [PATCH v5 1/2] crypto: skcipher AF_ALG - overhaul memory management
From: Stephan Müller @ 2017-03-30 13:59 UTC (permalink / raw)
To: Herbert Xu; +Cc: linux-crypto
In-Reply-To: <20170316095248.GA11996@gondor.apana.org.au>
[-- Attachment #1: Type: text/plain, Size: 1650 bytes --]
Am Donnerstag, 16. März 2017, 10:52:48 CEST schrieb Herbert Xu:
Hi Herbert,
> More importantly, with the current code, a very large recvmsg
> would still work by doing it piecemeal. With your patch, won't
> it fail because sock_kmalloc could fail to allocate memory for
> the whole thing?
For testing purpose, I wrote the app that is attached. It simply encrypts a
given file with ctr(aes).
On the current implementation of algif_skcipher where I directly pipe in the
provided memory block (i.e. using the stream operation of libkcapi):
dd if=/dev/zero of=testfile bs=1024 count=1000000
./kcapi-long testfile testfile.out
encryption failed with error -14
==> The -EFAULT happens at sendmsg() -- i.e. you cannot provide as much data
as you want.
==> On the proposed update, I see the same error.
When I use vmsplice, the current implementation somehow simply waits (I do not
yet know what happens). My new implementation simply returns the amount of
data that could have been spliced (64k -- which would allow user space to
implement a loop to send chunks).
When I ask libkcapi to send the data in chunks, libkcapi implements the loop
that sends/receives the data. In this case, both implementations of
algif_skcipher.c work to encrypt the whole file regardless of its size.
Thus, I would conclude that the current outer loop in recvmsg of the current
algif_skcipher is not really helpful as the bottleneck is on the sendmsg side.
With this, I would conclude that the new implementation of algif_skcipher.c
proposed in this patch set has the same behavior as the old one.
Ciao
Stephan
[-- Attachment #2: kcapi-long.c --]
[-- Type: text/x-csrc, Size: 4263 bytes --]
/*
* Copyright (C) 2017, Stephan Mueller <smueller@chronox.de>
*
* License: see LICENSE file in root directory
*
* THIS SOFTWARE IS PROVIDED ``AS IS'' AND ANY EXPRESS OR IMPLIED
* WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
* OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE, ALL OF
* WHICH ARE HEREBY DISCLAIMED. IN NO EVENT SHALL THE AUTHOR BE
* LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
* CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT
* OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR
* BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
* LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
* (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE
* USE OF THIS SOFTWARE, EVEN IF NOT ADVISED OF THE POSSIBILITY OF SUCH
* DAMAGE.
*/
#include <stdio.h>
#include <sys/mman.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <unistd.h>
#include <stdint.h>
#include <errno.h>
#include <string.h>
#include <fcntl.h>
#include <kcapi.h>
static int check_filetype(int fd, struct stat *sb, const char *filename)
{
fstat(fd, sb);
/* Do not return an error in case we cannot validate the data. */
if ((sb->st_mode & S_IFMT) != S_IFREG &&
(sb->st_mode & S_IFMT) != S_IFLNK) {
fprintf(stderr, "%s is no regular file or symlink\n", filename);
return -EINVAL;
}
return 0;
}
static int crypt(struct kcapi_handle *handle, const uint8_t *iv,
const char *infile, const char *outfile)
{
int infd = -1, outfd = -1;
int ret = 0;
struct stat insb, outsb;
uint8_t *inmem = NULL, *outmem = NULL;
size_t outsize;
infd = open(infile, O_RDONLY | O_CLOEXEC);
if (infd < 0) {
fprintf(stderr, "Cannot open file %s: %s\n", infile,
strerror(errno));
return -EIO;
}
outfd = open(outfile, O_RDWR | O_CLOEXEC | O_CREAT, S_IRWXU | S_IRWXG | S_IRWXO);
if (outfd < 0) {
fprintf(stderr, "Cannot open file %s: %s\n", infile,
strerror(errno));
ret = -EIO;
goto out;
}
ret = check_filetype(infd, &insb, infile);
if (ret)
goto out;
ret = check_filetype(outfd, &outsb, outfile);
if (ret)
goto out;
if (insb.st_size) {
inmem = mmap(NULL, insb.st_size, PROT_READ, MAP_SHARED,
infd, 0);
if (inmem == MAP_FAILED)
{
fprintf(stderr, "Use of mmap failed\n");
ret = -ENOMEM;
goto out;
}
}
outsize = ((insb.st_size + kcapi_cipher_blocksize(handle) - 1) /
kcapi_cipher_blocksize(handle)) *
kcapi_cipher_blocksize(handle);
ret = ftruncate(outfd, outsize);
if (ret)
goto out;
if (outsize) {
outmem = mmap(NULL, outsize, PROT_WRITE, MAP_SHARED,
outfd, 0);
if (outmem == MAP_FAILED)
{
fprintf(stderr, "Use of mmap failed\n");
ret = -ENOMEM;
goto out;
}
}
#if 1
/* Send all data in one go, libkcapi will not loop */
struct iovec iniov, outiov;
iniov.iov_base = inmem;
iniov.iov_len = insb.st_size;
outiov.iov_base = outmem;
outiov.iov_len = outsize;
ret = kcapi_cipher_stream_init_enc(handle, iv, NULL, 0);
if (ret)
goto out;
ret = kcapi_cipher_stream_update(handle, &iniov, 1);
if (ret)
goto out;
ret = kcapi_cipher_stream_op(handle, &outiov, 1);
#else
/* libkcapi will loop over the data and send it in chunks */
ret = kcapi_cipher_encrypt(handle, inmem, insb.st_size, iv,
outmem, outsize, KCAPI_ACCESS_SENDMSG);
#endif
out:
if (inmem && inmem != MAP_FAILED)
munmap(inmem, insb.st_size);
if (outmem && outmem != MAP_FAILED)
munmap(outmem, outsize);
if (infd >= 0)
close(infd);
if (outfd >= 0)
close(outfd);
return ret;
}
int main(int argc, char *argv[])
{
struct kcapi_handle *handle = NULL;
int ret;
if (argc != 3) {
fprintf(stderr, "infile, outfile required\n");
return -EINVAL;
}
/* with CTR mode, we can skip any padding */
ret = kcapi_cipher_init(&handle, "ctr(aes)", 0);
if (ret)
return ret;
ret = kcapi_cipher_setkey(handle, (uint8_t *)"0123456789012345", 16);
if (ret)
goto out;
ret = crypt(handle, (uint8_t *)"0123456789012345", argv[1], argv[2]);
if (ret > 0) {
fprintf(stderr, "%d bytes of ciphertext created\n", ret);
ret = 0;
} else {
fprintf(stderr, "encryption failed with error %d\n", ret);
}
out:
kcapi_cipher_destroy(handle);
return ret;
}
^ permalink raw reply
* Re: [PATCH 1/6] virtio: wrap find_vqs
From: Michael S. Tsirkin @ 2017-03-30 14:32 UTC (permalink / raw)
To: Jason Wang
Cc: Dmitry Tarnyagin, kvm, linux-remoteproc, dri-devel,
Bjorn Andersson, Gerd Hoffmann, James E.J. Bottomley, Herbert Xu,
linux-scsi, John Fastabend, Gonglei, Ohad Ben-Cohen,
Arnd Bergmann, Amit Shah, Stefan Hajnoczi, virtualization,
Martin K. Petersen, Greg Kroah-Hartman, linux-kernel,
linux-crypto, netdev, David S. Miller
In-Reply-To: <72f533a2-482d-1956-b0fe-254e273e1818@redhat.com>
On Thu, Mar 30, 2017 at 02:00:08PM +0800, Jason Wang wrote:
>
>
> On 2017年03月30日 04:48, Michael S. Tsirkin wrote:
> > We are going to add more parameters to find_vqs, let's wrap the call so
> > we don't need to tweak all drivers every time.
> >
> > Signed-off-by: Michael S. Tsirkin<mst@redhat.com>
> > ---
>
> A quick glance and it looks ok, but what the benefit of this series, is it
> required by other changes?
>
> Thanks
Yes - to avoid touching all devices when doing the rest of
the patchset.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [PATCH] crypto: vmx: Remove dubiously licensed crypto code
From: Paulo Flabiano Smorigo @ 2017-03-30 16:30 UTC (permalink / raw)
To: Tyrel Datwyler
Cc: Michal Suchánek, Greg Kroah-Hartman, Leonidas S. Barbosa,
Herbert Xu, Geert Uytterhoeven, linux-kernel, Paul Mackerras,
linux-crypto, Mauro Carvalho Chehab, linuxppc-dev,
David S. Miller
In-Reply-To: <7ec54553-610c-a5dc-d4d9-3c83f6a161d9@linux.vnet.ibm.com>
On 2017-03-29 20:08, Tyrel Datwyler wrote:
> On 03/29/2017 08:13 AM, Michal Suchánek wrote:
>> On Wed, 29 Mar 2017 16:51:35 +0200
>> Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:
>>
>>> On Wed, Mar 29, 2017 at 02:56:39PM +0200, Michal Suchanek wrote:
>>>> While reviewing commit 11c6e16ee13a ("crypto: vmx - Adding asm
>>>> subroutines for XTS") which adds the OpenSSL license header to
>>>> drivers/crypto/vmx/aesp8-ppc.pl licensing of this driver came into
>>>> qestion. The whole license reads:
>>>>
>>>> # Licensed under the OpenSSL license (the "License"). You may not
>>>> use # this file except in compliance with the License. You can
>>>> obtain a # copy
>>>> # in the file LICENSE in the source distribution or at
>>>> # https://www.openssl.org/source/license.html
>>>>
>>>> #
>>>> #
>>>> ====================================================================
>>>> # Written by Andy Polyakov <appro@openssl.org> for the OpenSSL #
>>>> project. The module is, however, dual licensed under OpenSSL and #
>>>> CRYPTOGAMS licenses depending on where you obtain it. For further #
>>>> details see http://www.openssl.org/~appro/cryptogams/. #
>>>> ====================================================================
>>>>
>>>> After seeking legal advice it is still not clear that this driver
>>>> can be legally used in Linux. In particular the "depending on where
>>>> you obtain it" part does not make it clear when you can apply the
>>>> GPL and when the OpenSSL license.
>>>>
>>>> I tried contacting the author of the code for clarification but did
>>>> not hear back. In absence of clear licensing the only solution I
>>>> see is removing this code.
>
> A quick 'git grep OpenSSL' of the Linux tree returns several other
> crypto files under the ARM architecture that are similarly licensed.
> Namely:
>
> arch/arm/crypto/sha1-armv4-large.S
> arch/arm/crypto/sha256-armv4.pl
> arch/arm/crypto/sha256-core.S_shipped
> arch/arm/crypto/sha512-armv4.pl
> arch/arm/crypto/sha512-core.S_shipped
> arch/arm64/crypto/sha256-core.S_shipped
> arch/arm64/crypto/sha512-armv8.pl
> arch/arm64/crypto/sha512-core.S_shipped
>
> On closer inspection of some of those files have the addendum that
> "Permission to use under GPL terms is granted", but not all of them.
>
> -Tyrel
In 2015, Andy Polyakov, the author, replied in this mailing list [1]:
"I have no problems with reusing assembly modules in kernel context. The
whole idea behind cryptogams initiative was exactly to reuse code in
different contexts."
[1] https://patchwork.kernel.org/patch/6027481/
--
Paulo Flabiano Smorigo
IBM Linux Technology Center
^ permalink raw reply
* [PATCH] crypto: gf128mul - define gf128mul_x_ble in gf128mul.h
From: Ondrej Mosnacek @ 2017-03-30 19:25 UTC (permalink / raw)
To: Herbert Xu; +Cc: David S. Miller, linux-crypto, Milan Broz, Ondrej Mosnacek
The gf128mul_x_ble function is currently defined in gf128mul.c, because
it depends on the gf128mul_table_be multiplication table.
However, since the function is very small and only uses two values from
the table, it is better for it to be defined as inline function in
gf128mul.h. That way, the function can be inlined by the compiler for
better performance.
After this change, the speed of the generic 'xts(aes)' implementation
increased from ~225 MiB/s to ~235 MiB/s (measured using 'cryptsetup
benchmark' on an Intel system with CRYPTO_AES_X86_64 and
CRYPTO_AES_NI_INTEL disabled).
Signed-off-by: Ondrej Mosnacek <omosnacek@gmail.com>
---
crypto/gf128mul.c | 11 -----------
include/crypto/gf128mul.h | 15 +++++++++++++--
2 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/crypto/gf128mul.c b/crypto/gf128mul.c
index 04facc0..2eab1a1 100644
--- a/crypto/gf128mul.c
+++ b/crypto/gf128mul.c
@@ -156,17 +156,6 @@ static void gf128mul_x_bbe(be128 *r, const be128 *x)
r->b = cpu_to_be64((b << 1) ^ _tt);
}
-void gf128mul_x_ble(be128 *r, const be128 *x)
-{
- u64 a = le64_to_cpu(x->a);
- u64 b = le64_to_cpu(x->b);
- u64 _tt = gf128mul_table_be[b >> 63];
-
- r->a = cpu_to_le64((a << 1) ^ _tt);
- r->b = cpu_to_le64((b << 1) | (a >> 63));
-}
-EXPORT_SYMBOL(gf128mul_x_ble);
-
static void gf128mul_x8_lle(be128 *x)
{
u64 a = be64_to_cpu(x->a);
diff --git a/include/crypto/gf128mul.h b/include/crypto/gf128mul.h
index 0bc9b5f..46a01a2 100644
--- a/include/crypto/gf128mul.h
+++ b/include/crypto/gf128mul.h
@@ -49,6 +49,7 @@
#ifndef _CRYPTO_GF128MUL_H
#define _CRYPTO_GF128MUL_H
+#include <asm/byteorder.h>
#include <crypto/b128ops.h>
#include <linux/slab.h>
@@ -163,8 +164,18 @@ void gf128mul_lle(be128 *a, const be128 *b);
void gf128mul_bbe(be128 *a, const be128 *b);
-/* multiply by x in ble format, needed by XTS */
-void gf128mul_x_ble(be128 *a, const be128 *b);
+/* Multiply by x in ble format, needed by XTS.
+ * Defined here for performance. */
+static inline void gf128mul_x_ble(be128 *r, const be128 *x)
+{
+ u64 a = le64_to_cpu(x->a);
+ u64 b = le64_to_cpu(x->b);
+ /* equivalent to gf128mul_table_be[b >> 63] (see crypto/gf128mul.c): */
+ u64 _tt = (b & ((u64)1 << 63)) ? 0x87 : 0x00;
+
+ r->a = cpu_to_le64((a << 1) ^ _tt);
+ r->b = cpu_to_le64((b << 1) | (a >> 63));
+}
/* 4k table optimization */
--
2.9.3
^ permalink raw reply related
* Re: [PATCH] crypto: gf128mul - define gf128mul_x_ble in gf128mul.h
From: Eric Biggers @ 2017-03-30 19:55 UTC (permalink / raw)
To: Ondrej Mosnacek; +Cc: Herbert Xu, David S. Miller, linux-crypto, Milan Broz
In-Reply-To: <20170330192535.23123-1-omosnacek@gmail.com>
Hi Ondrej,
On Thu, Mar 30, 2017 at 09:25:35PM +0200, Ondrej Mosnacek wrote:
> The gf128mul_x_ble function is currently defined in gf128mul.c, because
> it depends on the gf128mul_table_be multiplication table.
>
> However, since the function is very small and only uses two values from
> the table, it is better for it to be defined as inline function in
> gf128mul.h. That way, the function can be inlined by the compiler for
> better performance.
>
> After this change, the speed of the generic 'xts(aes)' implementation
> increased from ~225 MiB/s to ~235 MiB/s (measured using 'cryptsetup
> benchmark' on an Intel system with CRYPTO_AES_X86_64 and
> CRYPTO_AES_NI_INTEL disabled).
>
> Signed-off-by: Ondrej Mosnacek <omosnacek@gmail.com>
...
>
> -/* multiply by x in ble format, needed by XTS */
> -void gf128mul_x_ble(be128 *a, const be128 *b);
> +/* Multiply by x in ble format, needed by XTS.
> + * Defined here for performance. */
> +static inline void gf128mul_x_ble(be128 *r, const be128 *x)
> +{
> + u64 a = le64_to_cpu(x->a);
> + u64 b = le64_to_cpu(x->b);
> + /* equivalent to gf128mul_table_be[b >> 63] (see crypto/gf128mul.c): */
> + u64 _tt = (b & ((u64)1 << 63)) ? 0x87 : 0x00;
> +
> + r->a = cpu_to_le64((a << 1) ^ _tt);
> + r->b = cpu_to_le64((b << 1) | (a >> 63));
> +}
>
> /* 4k table optimization */
>
> --
> 2.9.3
>
This is an improvement; I'm just thinking that maybe this should be done for all
the gf128mul_x_*() functions, if only so that they use a consistent style and
are all defined next to each other.
Also note that '(b & ((u64)1 << 63)) ? 0x87 : 0x00;' is actually getting
compiled as '((s64)b >> 63) & 0x87', which is branchless and therefore makes the
new version more efficient than one might expect:
sar $0x3f,%rax
and $0x87,%eax
It could even be written the branchless way explicitly, but it shouldn't matter.
- Eric
^ permalink raw reply
* Re: [PATCH] crypto: gf128mul - define gf128mul_x_ble in gf128mul.h
From: Ondrej Mosnáček @ 2017-03-30 21:32 UTC (permalink / raw)
To: Eric Biggers; +Cc: Herbert Xu, David S. Miller, linux-crypto, Milan Broz
In-Reply-To: <20170330195546.GA60896@gmail.com>
Hi Eric,
2017-03-30 21:55 GMT+02:00 Eric Biggers <ebiggers3@gmail.com>:
> This is an improvement; I'm just thinking that maybe this should be done for all
> the gf128mul_x_*() functions, if only so that they use a consistent style and
> are all defined next to each other.
Right, that doesn't seem to be a bad idea... I was confused for a
while by the '& 0xff' in the _lle one, but now I see it also uses just
two values of the table, so it can be re-written in a similar way. In
fact, the OCB mode from RFC 7253 (that I'm currently trying to port to
kernel crypto API) uses gf128mul_x_bbe, so it would be useful to have
that one accessible, too.
I will move them all in v2, then.
> Also note that '(b & ((u64)1 << 63)) ? 0x87 : 0x00;' is actually getting
> compiled as '((s64)b >> 63) & 0x87', which is branchless and therefore makes the
> new version more efficient than one might expect:
>
> sar $0x3f,%rax
> and $0x87,%eax
>
> It could even be written the branchless way explicitly, but it shouldn't matter.
I think the definition using unsigned operations is more intuitive...
Let's just leave the clever tricks up to the compiler :)
Thanks,
O.M.
>
> - Eric
^ permalink raw reply
* [PATCH v2] crypto: gf128mul - define gf128mul_x_* in gf128mul.h
From: Ondrej Mosnacek @ 2017-03-30 22:04 UTC (permalink / raw)
To: Herbert Xu
Cc: David S. Miller, linux-crypto, Milan Broz, Ondrej Mosnacek,
Eric Biggers
The gf128mul_x_ble function is currently defined in gf128mul.c, because
it depends on the gf128mul_table_be multiplication table.
However, since the function is very small and only uses two values from
the table, it is better for it to be defined as inline function in
gf128mul.h. That way, the function can be inlined by the compiler for
better performance.
For consistency, the other gf128mul_x_* functions are also moved to the
header file.
After this change, the speed of the generic 'xts(aes)' implementation
increased from ~225 MiB/s to ~235 MiB/s (measured using 'cryptsetup
benchmark -c aes-xts-plain64' on an Intel system with CRYPTO_AES_X86_64
and CRYPTO_AES_NI_INTEL disabled).
Signed-off-by: Ondrej Mosnacek <omosnacek@gmail.com>
Cc: Eric Biggers <ebiggers@google.com>
---
crypto/gf128mul.c | 33 +------------------------------
include/crypto/gf128mul.h | 49 +++++++++++++++++++++++++++++++++++++++++++++--
2 files changed, 48 insertions(+), 34 deletions(-)
diff --git a/crypto/gf128mul.c b/crypto/gf128mul.c
index 04facc0..dc01212 100644
--- a/crypto/gf128mul.c
+++ b/crypto/gf128mul.c
@@ -130,43 +130,12 @@ static const u16 gf128mul_table_le[256] = gf128mul_dat(xda_le);
static const u16 gf128mul_table_be[256] = gf128mul_dat(xda_be);
/*
- * The following functions multiply a field element by x or by x^8 in
+ * The following functions multiply a field element by x^8 in
* the polynomial field representation. They use 64-bit word operations
* to gain speed but compensate for machine endianness and hence work
* correctly on both styles of machine.
*/
-static void gf128mul_x_lle(be128 *r, const be128 *x)
-{
- u64 a = be64_to_cpu(x->a);
- u64 b = be64_to_cpu(x->b);
- u64 _tt = gf128mul_table_le[(b << 7) & 0xff];
-
- r->b = cpu_to_be64((b >> 1) | (a << 63));
- r->a = cpu_to_be64((a >> 1) ^ (_tt << 48));
-}
-
-static void gf128mul_x_bbe(be128 *r, const be128 *x)
-{
- u64 a = be64_to_cpu(x->a);
- u64 b = be64_to_cpu(x->b);
- u64 _tt = gf128mul_table_be[a >> 63];
-
- r->a = cpu_to_be64((a << 1) | (b >> 63));
- r->b = cpu_to_be64((b << 1) ^ _tt);
-}
-
-void gf128mul_x_ble(be128 *r, const be128 *x)
-{
- u64 a = le64_to_cpu(x->a);
- u64 b = le64_to_cpu(x->b);
- u64 _tt = gf128mul_table_be[b >> 63];
-
- r->a = cpu_to_le64((a << 1) ^ _tt);
- r->b = cpu_to_le64((b << 1) | (a >> 63));
-}
-EXPORT_SYMBOL(gf128mul_x_ble);
-
static void gf128mul_x8_lle(be128 *x)
{
u64 a = be64_to_cpu(x->a);
diff --git a/include/crypto/gf128mul.h b/include/crypto/gf128mul.h
index 0bc9b5f..2a24553 100644
--- a/include/crypto/gf128mul.h
+++ b/include/crypto/gf128mul.h
@@ -49,6 +49,7 @@
#ifndef _CRYPTO_GF128MUL_H
#define _CRYPTO_GF128MUL_H
+#include <asm/byteorder.h>
#include <crypto/b128ops.h>
#include <linux/slab.h>
@@ -163,8 +164,52 @@ void gf128mul_lle(be128 *a, const be128 *b);
void gf128mul_bbe(be128 *a, const be128 *b);
-/* multiply by x in ble format, needed by XTS */
-void gf128mul_x_ble(be128 *a, const be128 *b);
+/*
+ * The following functions multiply a field element by x in
+ * the polynomial field representation. They use 64-bit word operations
+ * to gain speed but compensate for machine endianness and hence work
+ * correctly on both styles of machine.
+ *
+ * They are defined here for performance.
+ */
+
+static inline void gf128mul_x_lle(be128 *r, const be128 *x)
+{
+ u64 a = be64_to_cpu(x->a);
+ u64 b = be64_to_cpu(x->b);
+
+ /* equivalent to gf128mul_table_le[(b << 7) & 0xff] >> 8
+ * (see crypto/gf128mul.c): */
+ u64 _tt = (b & (u64)1) ? 0xe1 : 0x00;
+
+ r->b = cpu_to_be64((b >> 1) | (a << 63));
+ r->a = cpu_to_be64((a >> 1) ^ (_tt << 56));
+}
+
+static inline void gf128mul_x_bbe(be128 *r, const be128 *x)
+{
+ u64 a = be64_to_cpu(x->a);
+ u64 b = be64_to_cpu(x->b);
+
+ /* equivalent to gf128mul_table_be[a >> 63] (see crypto/gf128mul.c): */
+ u64 _tt = (a & ((u64)1 << 63)) ? 0x87 : 0x00;
+
+ r->a = cpu_to_be64((a << 1) | (b >> 63));
+ r->b = cpu_to_be64((b << 1) ^ _tt);
+}
+
+/* needed by XTS */
+static inline void gf128mul_x_ble(be128 *r, const be128 *x)
+{
+ u64 a = le64_to_cpu(x->a);
+ u64 b = le64_to_cpu(x->b);
+
+ /* equivalent to gf128mul_table_be[b >> 63] (see crypto/gf128mul.c): */
+ u64 _tt = (b & ((u64)1 << 63)) ? 0x87 : 0x00;
+
+ r->a = cpu_to_le64((a << 1) ^ _tt);
+ r->b = cpu_to_le64((b << 1) | (a >> 63));
+}
/* 4k table optimization */
--
2.9.3
^ permalink raw reply related
* Re: [PATCH v2] crypto: gf128mul - define gf128mul_x_* in gf128mul.h
From: Eric Biggers @ 2017-03-31 3:53 UTC (permalink / raw)
To: Ondrej Mosnacek
Cc: Herbert Xu, David S. Miller, linux-crypto, Milan Broz,
Eric Biggers
In-Reply-To: <20170330220442.11012-1-omosnacek@gmail.com>
On Fri, Mar 31, 2017 at 12:04:42AM +0200, Ondrej Mosnacek wrote:
> The gf128mul_x_ble function is currently defined in gf128mul.c, because
> it depends on the gf128mul_table_be multiplication table.
>
> However, since the function is very small and only uses two values from
> the table, it is better for it to be defined as inline function in
> gf128mul.h. That way, the function can be inlined by the compiler for
> better performance.
>
> For consistency, the other gf128mul_x_* functions are also moved to the
> header file.
>
> After this change, the speed of the generic 'xts(aes)' implementation
> increased from ~225 MiB/s to ~235 MiB/s (measured using 'cryptsetup
> benchmark -c aes-xts-plain64' on an Intel system with CRYPTO_AES_X86_64
> and CRYPTO_AES_NI_INTEL disabled).
>
Reviewed-by: Eric Biggers <ebiggers@google.com>
Thanks,
- Eric
^ permalink raw reply
* Re: [PATCH 1/6] virtio: wrap find_vqs
From: Jason Wang @ 2017-03-31 4:04 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: linux-kernel, John Fastabend, Arnd Bergmann, Greg Kroah-Hartman,
Amit Shah, Gonglei, Herbert Xu, David S. Miller, David Airlie,
Gerd Hoffmann, Dmitry Tarnyagin, Ohad Ben-Cohen, Bjorn Andersson,
James E.J. Bottomley, Martin K. Petersen, Stefan Hajnoczi,
virtualization, linux-crypto, dri-devel, netdev
In-Reply-To: <20170330173146-mutt-send-email-mst@kernel.org>
On 2017年03月30日 22:32, Michael S. Tsirkin wrote:
> On Thu, Mar 30, 2017 at 02:00:08PM +0800, Jason Wang wrote:
>>
>> On 2017年03月30日 04:48, Michael S. Tsirkin wrote:
>>> We are going to add more parameters to find_vqs, let's wrap the call so
>>> we don't need to tweak all drivers every time.
>>>
>>> Signed-off-by: Michael S. Tsirkin<mst@redhat.com>
>>> ---
>> A quick glance and it looks ok, but what the benefit of this series, is it
>> required by other changes?
>>
>> Thanks
> Yes - to avoid touching all devices when doing the rest of
> the patchset.
Maybe I'm not clear. I mean the benefit of this series not this single
patch. I guess it may be used by you proposal that avoid reset when set
XDP? If yes, do we really want to drop some packets after XDP is set?
Thanks
^ permalink raw reply
* Re: [PATCH] crypto: gf128mul - define gf128mul_x_ble in gf128mul.h
From: Jeffrey Walton @ 2017-03-31 6:05 UTC (permalink / raw)
To: Ondrej Mosnáček; +Cc: linux-crypto
In-Reply-To: <CAAUqJDs37N7NETK2DWU5mpV3ZwnYbi399bEpL9HHg8fRawch+A@mail.gmail.com>
>> Also note that '(b & ((u64)1 << 63)) ? 0x87 : 0x00;' is actually getting
>> compiled as '((s64)b >> 63) & 0x87', which is branchless and therefore makes the
>> new version more efficient than one might expect:
>>
>> sar $0x3f,%rax
>> and $0x87,%eax
>>
>> It could even be written the branchless way explicitly, but it shouldn't matter.
>
> I think the definition using unsigned operations is more intuitive...
> Let's just leave the clever tricks up to the compiler :)
It may be a good idea to use the one that provides constant time-ness
to help avoid leaking information.
Jeff
^ permalink raw reply
* Re: [PATCH] crypto: gf128mul - define gf128mul_x_ble in gf128mul.h
From: Ondrej Mosnáček @ 2017-03-31 9:17 UTC (permalink / raw)
To: noloader; +Cc: linux-crypto
In-Reply-To: <CAH8yC8nNJZ9yhvffkz7Fqg=zXFJTA+VQohAWa=8RFkpMBsY_qQ@mail.gmail.com>
Hi Jeff,
2017-03-31 8:05 GMT+02:00 Jeffrey Walton <noloader@gmail.com>:
>>> Also note that '(b & ((u64)1 << 63)) ? 0x87 : 0x00;' is actually getting
>>> compiled as '((s64)b >> 63) & 0x87', which is branchless and therefore makes the
>>> new version more efficient than one might expect:
>>>
>>> sar $0x3f,%rax
>>> and $0x87,%eax
>>>
>>> It could even be written the branchless way explicitly, but it shouldn't matter.
>>
>> I think the definition using unsigned operations is more intuitive...
>> Let's just leave the clever tricks up to the compiler :)
>
> It may be a good idea to use the one that provides constant time-ness
> to help avoid leaking information.
That's a good point... I played around with various ways to write the
expression in Compiler Explorer [1] and indeed GCC fails to produce
constant-time code from my version on some architectures (e.g. the
32-bit ARM). The version with an explicit arithmetic right shift seems
to produce the most efficient code across platforms, so I'll rewrite
it like that for v3.
Thanks,
O.M.
[1] https://gcc.godbolt.org/
^ permalink raw reply
* [PATCH v3] crypto: gf128mul - define gf128mul_x_* in gf128mul.h
From: Ondrej Mosnacek @ 2017-03-31 9:27 UTC (permalink / raw)
To: Herbert Xu
Cc: David S. Miller, linux-crypto, Jeffrey Walton, Milan Broz,
Ondrej Mosnacek, Eric Biggers
The gf128mul_x_ble function is currently defined in gf128mul.c, because
it depends on the gf128mul_table_be multiplication table.
However, since the function is very small and only uses two values from
the table, it is better for it to be defined as inline function in
gf128mul.h. That way, the function can be inlined by the compiler for
better performance.
For consistency, the other gf128mul_x_* functions are also moved to the
header file. In addition, the code is rewritten to be constant-time.
After this change, the speed of the generic 'xts(aes)' implementation
increased from ~225 MiB/s to ~235 MiB/s (measured using 'cryptsetup
benchmark -c aes-xts-plain64' on an Intel system with CRYPTO_AES_X86_64
and CRYPTO_AES_NI_INTEL disabled).
Signed-off-by: Ondrej Mosnacek <omosnacek@gmail.com>
Cc: Eric Biggers <ebiggers@google.com>
---
v2 -> v3: constant-time implementation
v1 -> v2: move all _x_ functions to the header, not just gf128mul_x_ble
crypto/gf128mul.c | 33 +---------------------------
include/crypto/gf128mul.h | 55 +++++++++++++++++++++++++++++++++++++++++++++--
2 files changed, 54 insertions(+), 34 deletions(-)
diff --git a/crypto/gf128mul.c b/crypto/gf128mul.c
index 04facc0..dc01212 100644
--- a/crypto/gf128mul.c
+++ b/crypto/gf128mul.c
@@ -130,43 +130,12 @@ static const u16 gf128mul_table_le[256] = gf128mul_dat(xda_le);
static const u16 gf128mul_table_be[256] = gf128mul_dat(xda_be);
/*
- * The following functions multiply a field element by x or by x^8 in
+ * The following functions multiply a field element by x^8 in
* the polynomial field representation. They use 64-bit word operations
* to gain speed but compensate for machine endianness and hence work
* correctly on both styles of machine.
*/
-static void gf128mul_x_lle(be128 *r, const be128 *x)
-{
- u64 a = be64_to_cpu(x->a);
- u64 b = be64_to_cpu(x->b);
- u64 _tt = gf128mul_table_le[(b << 7) & 0xff];
-
- r->b = cpu_to_be64((b >> 1) | (a << 63));
- r->a = cpu_to_be64((a >> 1) ^ (_tt << 48));
-}
-
-static void gf128mul_x_bbe(be128 *r, const be128 *x)
-{
- u64 a = be64_to_cpu(x->a);
- u64 b = be64_to_cpu(x->b);
- u64 _tt = gf128mul_table_be[a >> 63];
-
- r->a = cpu_to_be64((a << 1) | (b >> 63));
- r->b = cpu_to_be64((b << 1) ^ _tt);
-}
-
-void gf128mul_x_ble(be128 *r, const be128 *x)
-{
- u64 a = le64_to_cpu(x->a);
- u64 b = le64_to_cpu(x->b);
- u64 _tt = gf128mul_table_be[b >> 63];
-
- r->a = cpu_to_le64((a << 1) ^ _tt);
- r->b = cpu_to_le64((b << 1) | (a >> 63));
-}
-EXPORT_SYMBOL(gf128mul_x_ble);
-
static void gf128mul_x8_lle(be128 *x)
{
u64 a = be64_to_cpu(x->a);
diff --git a/include/crypto/gf128mul.h b/include/crypto/gf128mul.h
index 0bc9b5f..6e43be5 100644
--- a/include/crypto/gf128mul.h
+++ b/include/crypto/gf128mul.h
@@ -49,6 +49,7 @@
#ifndef _CRYPTO_GF128MUL_H
#define _CRYPTO_GF128MUL_H
+#include <asm/byteorder.h>
#include <crypto/b128ops.h>
#include <linux/slab.h>
@@ -163,8 +164,58 @@ void gf128mul_lle(be128 *a, const be128 *b);
void gf128mul_bbe(be128 *a, const be128 *b);
-/* multiply by x in ble format, needed by XTS */
-void gf128mul_x_ble(be128 *a, const be128 *b);
+/*
+ * The following functions multiply a field element by x in
+ * the polynomial field representation. They use 64-bit word operations
+ * to gain speed but compensate for machine endianness and hence work
+ * correctly on both styles of machine.
+ *
+ * They are defined here for performance.
+ */
+
+static inline u64 gf128mul_mask_from_bit(u64 x, int which)
+{
+ /* a constant-time version of 'x & ((u64)1 << which) ? (u64)-1 : 0' */
+ return ((s64)(x << (63 - which)) >> 63);
+}
+
+static inline void gf128mul_x_lle(be128 *r, const be128 *x)
+{
+ u64 a = be64_to_cpu(x->a);
+ u64 b = be64_to_cpu(x->b);
+
+ /* equivalent to gf128mul_table_le[(b << 7) & 0xff] >> 8
+ * (see crypto/gf128mul.c): */
+ u64 _tt = gf128mul_mask_from_bit(b, 0) & 0xe1;
+
+ r->b = cpu_to_be64((b >> 1) | (a << 63));
+ r->a = cpu_to_be64((a >> 1) ^ (_tt << 56));
+}
+
+static inline void gf128mul_x_bbe(be128 *r, const be128 *x)
+{
+ u64 a = be64_to_cpu(x->a);
+ u64 b = be64_to_cpu(x->b);
+
+ /* equivalent to gf128mul_table_be[a >> 63] (see crypto/gf128mul.c): */
+ u64 _tt = gf128mul_mask_from_bit(a, 63) & 0x87;
+
+ r->a = cpu_to_be64((a << 1) | (b >> 63));
+ r->b = cpu_to_be64((b << 1) ^ _tt);
+}
+
+/* needed by XTS */
+static inline void gf128mul_x_ble(be128 *r, const be128 *x)
+{
+ u64 a = le64_to_cpu(x->a);
+ u64 b = le64_to_cpu(x->b);
+
+ /* equivalent to gf128mul_table_be[b >> 63] (see crypto/gf128mul.c): */
+ u64 _tt = gf128mul_mask_from_bit(b, 63) & 0x87;
+
+ r->a = cpu_to_le64((a << 1) ^ _tt);
+ r->b = cpu_to_le64((b << 1) | (a >> 63));
+}
/* 4k table optimization */
--
2.9.3
^ permalink raw reply related
* Crypto Fixes for 4.11
From: Herbert Xu @ 2017-03-31 10:29 UTC (permalink / raw)
To: Linus Torvalds, David S. Miller, Linux Kernel Mailing List,
Linux Crypto Mailing List
In-Reply-To: <20170324134627.GA14273@gondor.apana.org.au>
Hi Linus:
This push fixes the following issues:
- Memory corruption when kmalloc fails in xts/lrw.
- Mark some CCP DMA channels as private.
- Fix reordering race in padata.
- Regression in omap-rng DT description.
Please pull from
git://git.kernel.org/pub/scm/linux/kernel/git/herbert/crypto-2.6.git linus
Eric Biggers (1):
crypto: xts,lrw - fix out-of-bounds write after kmalloc failure
Gary R Hook (1):
crypto: ccp - Make some CCP DMA channels private
Jason A. Donenfeld (1):
padata: avoid race in reordering
Thomas Petazzoni (1):
dt-bindings: rng: clocks property on omap_rng not always mandatory
Documentation/devicetree/bindings/rng/omap_rng.txt | 3 +-
crypto/lrw.c | 7 +++-
crypto/xts.c | 7 +++-
drivers/crypto/ccp/ccp-dev-v5.c | 1 +
drivers/crypto/ccp/ccp-dev.h | 5 +++
drivers/crypto/ccp/ccp-dmaengine.c | 41 ++++++++++++++++++++
kernel/padata.c | 5 ++-
7 files changed, 62 insertions(+), 7 deletions(-)
Thanks,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH v5 1/2] crypto: skcipher AF_ALG - overhaul memory management
From: Herbert Xu @ 2017-03-31 10:33 UTC (permalink / raw)
To: Stephan Müller; +Cc: linux-crypto
In-Reply-To: <4808097.uXnqYIEybL@tauon.atsec.com>
On Thu, Mar 16, 2017 at 11:18:33AM +0100, Stephan Müller wrote:
> Am Donnerstag, 16. März 2017, 10:52:48 CET schrieb Herbert Xu:
>
> Hi Herbert,
>
> > First of all you're only limiting the amount of memory occupied
> > by the SG list which is not the same thing as the memory pinned
> > down by the actual recvmsg.
>
> I am fully aware of that. As this was present in the code, I thought I could
> reuse that approach.
>
> Are you saying that you want to stop this approach?
No you're confusing things. Previously there was an explicit
limit on the number of pages that can be pinned. Now you're
only indirectly limiting it by limiting the size of the metadata
through sock_kmalloc.
The end result is that you're now allowing a huge amount of user
memory to be pinned down by the system call. This is *unacceptable*.
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH] OF: mark released devices as no longer populated
From: Horia Geantă @ 2017-03-31 15:23 UTC (permalink / raw)
To: Russell King - ARM Linux, Rob Herring, Fabio Estevam
Cc: Frank Rowand, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
linux-crypto-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Dan Douglass
In-Reply-To: <20170331103950.GA18376@n2100.armlinux.org.uk>
On 3/31/2017 1:40 PM, Russell King - ARM Linux wrote:
> Ping, this issue still exists with 4.11-rc4 - and there's been no
> reaction from the alleged CAAM maintainers.
>
Sorry, this somehow slipped through (Cc vs. To, no linux-crypto).
> On Tue, Aug 09, 2016 at 11:48:38AM -0500, Rob Herring wrote:
>> On Tue, Aug 9, 2016 at 4:33 AM, Russell King <rmk+kernel-I+IVW8TIWO2tmTQ+vhA3Yw@public.gmane.org> wrote:
>>> When a Linux device is released and cleaned up, we left the OF device
>>> node marked as populated. This causes the Freescale CAAM driver
>>> (drivers/crypto/caam) problems when the module is removed and re-
>>> inserted:
>>>
>>> JR0 Platform device creation error
>>> JR0 Platform device creation error
>>> caam 2100000.caam: no queues configured, terminating
>>> caam: probe of 2100000.caam failed with error -12
>>>
>>> The reason is that CAAM creates platform devices for each job ring:
>>>
>>> for_each_available_child_of_node(nprop, np)
>>> if (of_device_is_compatible(np, "fsl,sec-v4.0-job-ring") ||
>>> of_device_is_compatible(np, "fsl,sec4.0-job-ring")) {
>>> ctrlpriv->jrpdev[ring] =
>>> of_platform_device_create(np, NULL, dev);
>>>
>>> which sets OF_POPULATED on the device node, but then it cleans these
>>> up:
>>>
>>> /* Remove platform devices for JobRs */
>>> for (ring = 0; ring < ctrlpriv->total_jobrs; ring++) {
>>> if (ctrlpriv->jrpdev[ring])
>>> of_device_unregister(ctrlpriv->jrpdev[ring]);
>>
>> This looks a bit asymmetrical to me with a of_platform_device_* call
>> and a of_device_* call.
>>
>> I think you could use of_platform_{de}populate here instead. That
>> would simplify things in the driver a bit too as you wouldn't need to
>> store jrpdev. It wouldn't work if there are other child nodes with
Indeed, this would clean-up the driver a bit. However, the driver needs
to know how many of the devices probed successfully - to print the
number and more importantly to exit in case total_jobrs = 0.
Thus, I would keep the one-by-one probing of the devices.
What options are there in this case?
Should a function symmetric to of_platform_device_create() be added - to
replace of_device_unregister() - or rely on an open-coded solution?
Thanks,
Horia
>> compatible strings which you don't want devices created.
>>
>>> }
>>>
>>> which leaves OF_POPULATED set.
>>>
>>> Arrange for platform devices with a device node to clear the
>>> OF_POPULATED bit when they are released.
>>>
>>> Signed-off-by: Russell King <rmk+kernel-I+IVW8TIWO2tmTQ+vhA3Yw@public.gmane.org>
>>> ---
>>> Please check this carefully - it may have issues where an of_node
>>> pointer is copied from one platform device to another, but IMHO
>>> doing that is itself buggy behaviour.
>>
>> Agreed, that is wrong.
>>
>>>
>>> Resending due to wrong list address, sorry.
>>>
>>> include/linux/of_device.h | 1 +
>>> 1 file changed, 1 insertion(+)
>>>
>>> diff --git a/include/linux/of_device.h b/include/linux/of_device.h
>>> index cc7dd687a89d..7a8362d0c6d2 100644
>>> --- a/include/linux/of_device.h
>>> +++ b/include/linux/of_device.h
>>> @@ -43,6 +43,7 @@ extern int of_device_uevent_modalias(struct device *dev, struct kobj_uevent_env
>>>
>>> static inline void of_device_node_put(struct device *dev)
>>> {
>>> + of_node_clear_flag(dev->of_node, OF_POPULATED);
>>
>> This would result in clearing the flag twice in the
>> of_platform_populate/of_platform_depopulate case. It would do the same
>> for other bus types like i2c as well. That doesn't really hurt
>> anything that I can think of, but just not the best implementation. I
>> think adding a of_platform_device_unregister() call that wraps
>> of_platform_device_destroy would be more balanced.
>>
>> I looked thru all the callers of of_platform_device_create. The only
>> other ones affected by this are:
>>
>> drivers/macintosh/ams/ams-core.c
>> drivers/macintosh/therm_adt746x.c
>> drivers/macintosh/therm_windtunnel.c
>>
>> The others either have no remove path or a buggy remove path.
>>
>> Rob
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 1/6] virtio: wrap find_vqs
From: Michael S. Tsirkin @ 2017-03-31 16:21 UTC (permalink / raw)
To: Jason Wang
Cc: linux-kernel, John Fastabend, Arnd Bergmann, Greg Kroah-Hartman,
Amit Shah, Gonglei, Herbert Xu, David S. Miller, David Airlie,
Gerd Hoffmann, Dmitry Tarnyagin, Ohad Ben-Cohen, Bjorn Andersson,
James E.J. Bottomley, Martin K. Petersen, Stefan Hajnoczi,
virtualization, linux-crypto, dri-devel, netdev
In-Reply-To: <c388e4fd-2956-8894-31ec-31fe8b01e49f@redhat.com>
On Fri, Mar 31, 2017 at 12:04:55PM +0800, Jason Wang wrote:
>
>
> On 2017年03月30日 22:32, Michael S. Tsirkin wrote:
> > On Thu, Mar 30, 2017 at 02:00:08PM +0800, Jason Wang wrote:
> > >
> > > On 2017年03月30日 04:48, Michael S. Tsirkin wrote:
> > > > We are going to add more parameters to find_vqs, let's wrap the call so
> > > > we don't need to tweak all drivers every time.
> > > >
> > > > Signed-off-by: Michael S. Tsirkin<mst@redhat.com>
> > > > ---
> > > A quick glance and it looks ok, but what the benefit of this series, is it
> > > required by other changes?
> > >
> > > Thanks
> > Yes - to avoid touching all devices when doing the rest of
> > the patchset.
>
> Maybe I'm not clear. I mean the benefit of this series not this single
> patch. I guess it may be used by you proposal that avoid reset when set XDP?
In particular, yes. It generally simplifies things significantly if
we can get the true buffer size back.
> If yes, do we really want to drop some packets after XDP is set?
>
> Thanks
We would rather not drop packets. We could detect and copy them to make
XDP work.
--
MST
^ permalink raw reply
* Re: [PATCH] OF: mark released devices as no longer populated
From: Rob Herring @ 2017-03-31 16:59 UTC (permalink / raw)
To: Horia Geantă
Cc: Russell King - ARM Linux, Fabio Estevam, Frank Rowand,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
linux-crypto-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Dan Douglass
In-Reply-To: <VI1PR0401MB2591E5FD962023914F042C9B98370-9IDQY6o3qQhGNIhRVge97I3W/0Ik+aLCnBOFsp37pqbUKgpGm//BTAC/G2K4zDHf@public.gmane.org>
On Fri, Mar 31, 2017 at 10:23 AM, Horia Geantă <horia.geanta-3arQi8VN3Tc@public.gmane.org> wrote:
> On 3/31/2017 1:40 PM, Russell King - ARM Linux wrote:
>> Ping, this issue still exists with 4.11-rc4 - and there's been no
>> reaction from the alleged CAAM maintainers.
>>
> Sorry, this somehow slipped through (Cc vs. To, no linux-crypto).
>
>> On Tue, Aug 09, 2016 at 11:48:38AM -0500, Rob Herring wrote:
>>> On Tue, Aug 9, 2016 at 4:33 AM, Russell King <rmk+kernel-I+IVW8TIWO2tmTQ+vhA3Yw@public.gmane.org> wrote:
>>>> When a Linux device is released and cleaned up, we left the OF device
>>>> node marked as populated. This causes the Freescale CAAM driver
>>>> (drivers/crypto/caam) problems when the module is removed and re-
>>>> inserted:
>>>>
>>>> JR0 Platform device creation error
>>>> JR0 Platform device creation error
>>>> caam 2100000.caam: no queues configured, terminating
>>>> caam: probe of 2100000.caam failed with error -12
>>>>
>>>> The reason is that CAAM creates platform devices for each job ring:
>>>>
>>>> for_each_available_child_of_node(nprop, np)
>>>> if (of_device_is_compatible(np, "fsl,sec-v4.0-job-ring") ||
>>>> of_device_is_compatible(np, "fsl,sec4.0-job-ring")) {
>>>> ctrlpriv->jrpdev[ring] =
>>>> of_platform_device_create(np, NULL, dev);
>>>>
>>>> which sets OF_POPULATED on the device node, but then it cleans these
>>>> up:
>>>>
>>>> /* Remove platform devices for JobRs */
>>>> for (ring = 0; ring < ctrlpriv->total_jobrs; ring++) {
>>>> if (ctrlpriv->jrpdev[ring])
>>>> of_device_unregister(ctrlpriv->jrpdev[ring]);
>>>
>>> This looks a bit asymmetrical to me with a of_platform_device_* call
>>> and a of_device_* call.
>>>
>>> I think you could use of_platform_{de}populate here instead. That
>>> would simplify things in the driver a bit too as you wouldn't need to
>>> store jrpdev. It wouldn't work if there are other child nodes with
> Indeed, this would clean-up the driver a bit. However, the driver needs
> to know how many of the devices probed successfully - to print the
> number and more importantly to exit in case total_jobrs = 0.
The only thing you are guaranteed is the OF code created some platform
devices. That's it. Whether any driver probed successfully is separate
and a lot more things can go wrong there. The only thing you are
checking is that your dtb is not crap.
> Thus, I would keep the one-by-one probing of the devices.
> What options are there in this case?
> Should a function symmetric to of_platform_device_create() be added - to
> replace of_device_unregister() - or rely on an open-coded solution?
Certainly not the latter. We don't want drivers mucking with flags
internal to the DT code.
Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v3] crypto: gf128mul - define gf128mul_x_* in gf128mul.h
From: Eric Biggers @ 2017-04-01 3:44 UTC (permalink / raw)
To: Ondrej Mosnacek
Cc: Herbert Xu, David S. Miller, linux-crypto, Jeffrey Walton,
Milan Broz, Eric Biggers
In-Reply-To: <20170331092703.2520-1-omosnacek@gmail.com>
On Fri, Mar 31, 2017 at 11:27:03AM +0200, Ondrej Mosnacek wrote:
> The gf128mul_x_ble function is currently defined in gf128mul.c, because
> it depends on the gf128mul_table_be multiplication table.
>
> However, since the function is very small and only uses two values from
> the table, it is better for it to be defined as inline function in
> gf128mul.h. That way, the function can be inlined by the compiler for
> better performance.
>
> For consistency, the other gf128mul_x_* functions are also moved to the
> header file. In addition, the code is rewritten to be constant-time.
>
> After this change, the speed of the generic 'xts(aes)' implementation
> increased from ~225 MiB/s to ~235 MiB/s (measured using 'cryptsetup
> benchmark -c aes-xts-plain64' on an Intel system with CRYPTO_AES_X86_64
> and CRYPTO_AES_NI_INTEL disabled).
>
> Signed-off-by: Ondrej Mosnacek <omosnacek@gmail.com>
> Cc: Eric Biggers <ebiggers@google.com>
Reviewed-by: Eric Biggers <ebiggers@google.com>
Also, I realized that for gf128mul_x_lle() now that we aren't using the table we
don't need to shift '_tt' but rather can use the constant 0xe100000000000000:
/* equivalent to (u64)gf128mul_table_le[(b << 7) & 0xff] << 48
* (see crypto/gf128mul.c): */
u64 _tt = gf128mul_mask_from_bit(b, 0) & 0xe100000000000000;
r->b = cpu_to_be64((b >> 1) | (a << 63));
r->a = cpu_to_be64((a >> 1) ^ _tt);
I think that would be better and you could send a v4 to do it that way if you
want. It's not a huge deal though.
Thanks!
- Eric
^ permalink raw reply
* [PATCH] crypto: AF_ALG: handle 0 lengths in af_alg_make_sg
From: Stephan Müller @ 2017-04-01 15:04 UTC (permalink / raw)
To: herbert; +Cc: linux-crypto
Hi Herbert,
If you concur with the patch, I think it should go to 4.11 as well as
to stable.
Ciao
Stephan
---8<---
The function af_alg_make_sg converts user-provided IOVECs into an SGL.
Thus it operates directly on the user-space provided number of IOVECs.
When user space provides 0 for the number of IOVECs iov_iter_get_pages
returns a bogus number of bytes. This in turn will cause a crash when
the SGL is processed.
The fix initializes an SGL with one entry for handling chaining
operation but does not contain data.
In addition, the patch changes variable type of len from int to size_t
to be consistent with the data type of the invoker and the data type
where len is used.
Signed-off-by: Stephan Mueller <smueller@chronox.de>
---
crypto/af_alg.c | 10 +++++++++-
include/crypto/if_alg.h | 2 +-
2 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/crypto/af_alg.c b/crypto/af_alg.c
index 24dc082..5992997 100644
--- a/crypto/af_alg.c
+++ b/crypto/af_alg.c
@@ -399,12 +399,20 @@ static const struct net_proto_family alg_family = {
.owner = THIS_MODULE,
};
-int af_alg_make_sg(struct af_alg_sgl *sgl, struct iov_iter *iter, int len)
+int af_alg_make_sg(struct af_alg_sgl *sgl, struct iov_iter *iter, size_t len)
{
size_t off;
ssize_t n;
int npages, i;
+ if (!len) {
+ /* init one for linking */
+ sg_init_table(sgl->sg, 1);
+ sg_mark_end(sgl->sg);
+ sgl->npages = 0;
+ return 0;
+ }
+
n = iov_iter_get_pages(iter, sgl->pages, len, ALG_MAX_PAGES, &off);
if (n < 0)
return n;
diff --git a/include/crypto/if_alg.h b/include/crypto/if_alg.h
index 6c3e6e7..c637ac9 100644
--- a/include/crypto/if_alg.h
+++ b/include/crypto/if_alg.h
@@ -76,7 +76,7 @@ int af_alg_release(struct socket *sock);
void af_alg_release_parent(struct sock *sk);
int af_alg_accept(struct sock *sk, struct socket *newsock);
-int af_alg_make_sg(struct af_alg_sgl *sgl, struct iov_iter *iter, int len);
+int af_alg_make_sg(struct af_alg_sgl *sgl, struct iov_iter *iter, size_t len);
void af_alg_free_sg(struct af_alg_sgl *sgl);
void af_alg_link_sg(struct af_alg_sgl *sgl_prev, struct af_alg_sgl *sgl_new);
--
2.9.3
^ permalink raw reply related
* Re: [PATCH v3] crypto: gf128mul - define gf128mul_x_* in gf128mul.h
From: Ondrej Mosnáček @ 2017-04-01 15:13 UTC (permalink / raw)
To: Eric Biggers
Cc: Herbert Xu, David S. Miller, linux-crypto, Jeffrey Walton,
Milan Broz, Eric Biggers
In-Reply-To: <20170401034407.GA598@zzz>
2017-04-01 5:44 GMT+02:00 Eric Biggers <ebiggers3@gmail.com>:
> Also, I realized that for gf128mul_x_lle() now that we aren't using the table we
> don't need to shift '_tt' but rather can use the constant 0xe100000000000000:
>
> /* equivalent to (u64)gf128mul_table_le[(b << 7) & 0xff] << 48
> * (see crypto/gf128mul.c): */
> u64 _tt = gf128mul_mask_from_bit(b, 0) & 0xe100000000000000;
>
> r->b = cpu_to_be64((b >> 1) | (a << 63));
> r->a = cpu_to_be64((a >> 1) ^ _tt);
>
> I think that would be better and you could send a v4 to do it that way if you
> want. It's not a huge deal though.
Yes, I was hoping the compiler would be wise enough to fold the shift
into the constant, but I didn't actually check the assembly output...
I took the time to write a quick benchmark and the version without
shift is indeed notably faster.
That said, I'll go the extra mile and send a v4.
Thanks for the review!
O.M.
^ permalink raw reply
* [PATCH v4] crypto: gf128mul - define gf128mul_x_* in gf128mul.h
From: Ondrej Mosnacek @ 2017-04-01 15:17 UTC (permalink / raw)
To: Herbert Xu
Cc: David S. Miller, linux-crypto, Jeffrey Walton, Milan Broz,
Ondrej Mosnacek, Eric Biggers
The gf128mul_x_ble function is currently defined in gf128mul.c, because
it depends on the gf128mul_table_be multiplication table.
However, since the function is very small and only uses two values from
the table, it is better for it to be defined as inline function in
gf128mul.h. That way, the function can be inlined by the compiler for
better performance.
For consistency, the other gf128mul_x_* functions are also moved to the
header file. In addition, the code is rewritten to be constant-time.
After this change, the speed of the generic 'xts(aes)' implementation
increased from ~225 MiB/s to ~235 MiB/s (measured using 'cryptsetup
benchmark -c aes-xts-plain64' on an Intel system with CRYPTO_AES_X86_64
and CRYPTO_AES_NI_INTEL disabled).
Signed-off-by: Ondrej Mosnacek <omosnacek@gmail.com>
Cc: Eric Biggers <ebiggers@google.com>
---
v3 -> v4: a faster version of gf128mul_x_lle
v2 -> v3: constant-time implementation
v1 -> v2: move all _x_ functions to the header, not just gf128mul_x_ble
crypto/gf128mul.c | 33 +---------------------------
include/crypto/gf128mul.h | 55 +++++++++++++++++++++++++++++++++++++++++++++--
2 files changed, 54 insertions(+), 34 deletions(-)
diff --git a/crypto/gf128mul.c b/crypto/gf128mul.c
index 04facc0..dc01212 100644
--- a/crypto/gf128mul.c
+++ b/crypto/gf128mul.c
@@ -130,43 +130,12 @@ static const u16 gf128mul_table_le[256] = gf128mul_dat(xda_le);
static const u16 gf128mul_table_be[256] = gf128mul_dat(xda_be);
/*
- * The following functions multiply a field element by x or by x^8 in
+ * The following functions multiply a field element by x^8 in
* the polynomial field representation. They use 64-bit word operations
* to gain speed but compensate for machine endianness and hence work
* correctly on both styles of machine.
*/
-static void gf128mul_x_lle(be128 *r, const be128 *x)
-{
- u64 a = be64_to_cpu(x->a);
- u64 b = be64_to_cpu(x->b);
- u64 _tt = gf128mul_table_le[(b << 7) & 0xff];
-
- r->b = cpu_to_be64((b >> 1) | (a << 63));
- r->a = cpu_to_be64((a >> 1) ^ (_tt << 48));
-}
-
-static void gf128mul_x_bbe(be128 *r, const be128 *x)
-{
- u64 a = be64_to_cpu(x->a);
- u64 b = be64_to_cpu(x->b);
- u64 _tt = gf128mul_table_be[a >> 63];
-
- r->a = cpu_to_be64((a << 1) | (b >> 63));
- r->b = cpu_to_be64((b << 1) ^ _tt);
-}
-
-void gf128mul_x_ble(be128 *r, const be128 *x)
-{
- u64 a = le64_to_cpu(x->a);
- u64 b = le64_to_cpu(x->b);
- u64 _tt = gf128mul_table_be[b >> 63];
-
- r->a = cpu_to_le64((a << 1) ^ _tt);
- r->b = cpu_to_le64((b << 1) | (a >> 63));
-}
-EXPORT_SYMBOL(gf128mul_x_ble);
-
static void gf128mul_x8_lle(be128 *x)
{
u64 a = be64_to_cpu(x->a);
diff --git a/include/crypto/gf128mul.h b/include/crypto/gf128mul.h
index 0bc9b5f..35ced9d 100644
--- a/include/crypto/gf128mul.h
+++ b/include/crypto/gf128mul.h
@@ -49,6 +49,7 @@
#ifndef _CRYPTO_GF128MUL_H
#define _CRYPTO_GF128MUL_H
+#include <asm/byteorder.h>
#include <crypto/b128ops.h>
#include <linux/slab.h>
@@ -163,8 +164,58 @@ void gf128mul_lle(be128 *a, const be128 *b);
void gf128mul_bbe(be128 *a, const be128 *b);
-/* multiply by x in ble format, needed by XTS */
-void gf128mul_x_ble(be128 *a, const be128 *b);
+/*
+ * The following functions multiply a field element by x in
+ * the polynomial field representation. They use 64-bit word operations
+ * to gain speed but compensate for machine endianness and hence work
+ * correctly on both styles of machine.
+ *
+ * They are defined here for performance.
+ */
+
+static inline u64 gf128mul_mask_from_bit(u64 x, int which)
+{
+ /* a constant-time version of 'x & ((u64)1 << which) ? (u64)-1 : 0' */
+ return ((s64)(x << (63 - which)) >> 63);
+}
+
+static inline void gf128mul_x_lle(be128 *r, const be128 *x)
+{
+ u64 a = be64_to_cpu(x->a);
+ u64 b = be64_to_cpu(x->b);
+
+ /* equivalent to gf128mul_table_le[(b << 7) & 0xff] << 48
+ * (see crypto/gf128mul.c): */
+ u64 _tt = gf128mul_mask_from_bit(b, 0) & ((u64)0xe1 << 56);
+
+ r->b = cpu_to_be64((b >> 1) | (a << 63));
+ r->a = cpu_to_be64((a >> 1) ^ _tt);
+}
+
+static inline void gf128mul_x_bbe(be128 *r, const be128 *x)
+{
+ u64 a = be64_to_cpu(x->a);
+ u64 b = be64_to_cpu(x->b);
+
+ /* equivalent to gf128mul_table_be[a >> 63] (see crypto/gf128mul.c): */
+ u64 _tt = gf128mul_mask_from_bit(a, 63) & 0x87;
+
+ r->a = cpu_to_be64((a << 1) | (b >> 63));
+ r->b = cpu_to_be64((b << 1) ^ _tt);
+}
+
+/* needed by XTS */
+static inline void gf128mul_x_ble(be128 *r, const be128 *x)
+{
+ u64 a = le64_to_cpu(x->a);
+ u64 b = le64_to_cpu(x->b);
+
+ /* equivalent to gf128mul_table_be[b >> 63] (see crypto/gf128mul.c): */
+ u64 _tt = gf128mul_mask_from_bit(b, 63) & 0x87;
+
+ r->a = cpu_to_le64((a << 1) ^ _tt);
+ r->b = cpu_to_le64((b << 1) | (a >> 63));
+}
/* 4k table optimization */
--
2.9.3
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox