[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 1/4] sha1sum: use AF_ALG when available
From: |
Matteo Croce |
Subject: |
Re: [PATCH 1/4] sha1sum: use AF_ALG when available |
Date: |
Mon, 23 Apr 2018 18:52:18 +0200 |
On Mon, Apr 23, 2018 at 5:07 PM, Tim Rühsen <address@hidden> wrote:
>
> On 04/23/2018 01:17 PM, Matteo Croce wrote:
>> +#include <config.h>
>> +
>> +#include "af_alg.h"
>> +
>> +/* from linux/include/linux/fs.h: (INT_MAX & PAGE_MASK) */
>> +#define MAX_RW_COUNT 0x7FFFF000
>> +#define BLOCKSIZE 32768
>> +
>> +int
>> +afalg_stream (FILE * stream, void *resblock, const char *alg, int hashlen)
>> +{
>> + struct sockaddr_alg salg = {
>> + .salg_family = AF_ALG,
>> + .salg_type = "hash",
>> + };
>> + int ret, cfd, ofd;
>> + static char buf[BLOCKSIZE];
>> + ssize_t size;
>> + struct stat st;
>> +
>> + strcpy((char *)salg.salg_name, alg);
>
> Better consider for size of salg.salg_name here.
>
Right. Will check it in v2.
>> + cfd = socket (AF_ALG, SOCK_SEQPACKET, 0);
>> + if (cfd < 0)
>> + return -EAFNOSUPPORT;
>
> What about moving salg initialization here ?
>
I think that contextual initialization it's a good trick to fill it
with zeroes avoiding a memset later.
What are the advantage of initializing the structure later? The effort
should be minimal in case of no AF_ALG support.
>> + ret = bind (cfd, (struct sockaddr *) &salg, sizeof (salg));
>> + if (ret < 0)
>> + {
>> + ret = -EAFNOSUPPORT;
>> + goto out_cfd;
>> + }
>> +
>> + ofd = accept (cfd, NULL, 0);
>> + if (ofd < 0)
>> + {
>> + ret = -EAFNOSUPPORT;
>> + goto out_ofd;
>> + }
>> +
>> + /* if file is a regular file, attempt sendfile() to pipe the data */
>> + if (!fstat(fileno(stream), &st) && S_ISREG(st.st_mode) &&
>> + st.st_size <= MAX_RW_COUNT)
>
> Is it possible to skip that MAX_RW_COUNT check on recent glibc ?
> From 'man sendfile':
> "
> The original Linux sendfile() system call was not designed to handle
> large file offsets. Consequently, Linux 2.4 added
> sendfile64(), with a wider type for the offset argument. The glibc
> sendfile() wrapper function transparently deals
> with the kernel differences.
> "
>
The advantage of sendfile64() over sendfile() is only that the offset
in the input file can be bigger.
I've tried it and it seems that MAX_RW_COUNT is an hard limit on every system:
$ strace -e read dd if=/dev/zero of=/dev/null bs=3G count=1
read(0, "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"...,
3221225472) = 2147479552
0+1 records in
0+1 records out
2147479552 bytes (2.1 GB, 2.0 GiB) copied, 0.221906 s, 9.7 GB/s
As reported in the manpage too:
"sendfile() will transfer at most 0x7ffff000 (2,147,479,552) bytes,
returning the number of bytes actually transferred. (This is true on
both 32-bit and 64-bit systems.)"
>> + {
>> + if (sendfile(ofd, fileno(stream), NULL, st.st_size) == -1)
>
> From 'man sendfile':
> "
> Note that a successful call to
> sendfile() may write fewer bytes than requested; the caller should be
> prepared to retry the call if there were unsent
> bytes. See also NOTES.
> "
>
>> + ret = -EIO;
>> + else
>> + ret = 0;
>> + } else {
>> + /* sendfile() not possible, do a classic read-write loop */
>> + while ((size = fread (buf, 1, sizeof (buf), stream)))
>> + {
>> + if (send (ofd, buf, size, size == sizeof (buf) ? MSG_MORE : 0) ==
>> -1)
>> + {
>> + ret = -EIO;
>> + goto out_ofd;
>> + }
>> + }
>> + }
>
I might be wrong here, but looking at the kernel code of the
sendfile64(), which actually calls do_splice_direct() that calls
splice_direct_to_actor(),
it can happen only with nonblocking sockets. I didn't managed to
trigger such behaviour myself, but more testing is welcome.
>
> Regards, Tim
>
Regards,
--
Matteo Croce
per aspera ad upstream