On 23.04.2018 18:52, Matteo Croce wrote:
> On Mon, Apr 23, 2018 at 5:07 PM, Tim Rühsen <tim.rueh...@gmx.de> 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.
You are right with the minimal overhead for a single call, but you can
never be sure about corner use cases.
The contextual init is good (though I remember gcc clears all the memory
anyways - so no benefit by no using memset),
and you can also use it after the first code line (C99, which th
econstruct already is).
>>> + 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
Thanks for testing - I didn't read the man page thoroughly (or
misunderstood it).
> 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,
Regards, Tim
