From 8fa8b4113c25b47937ddf7cadf64f060d79c2ef5 Mon Sep 17 00:00:00 2001 From: Frank Lahm Date: Fri, 27 Apr 2012 14:23:42 +0200 Subject: [PATCH] Performance tuning: sendfilev, MSG_MORE and do not apply AFP read locks by default sendfilev: use sendfilev for data transfer MSG_MORE: sending DSI header with MSG_MORE before sendfile New option "afp read locks", default:no --- etc/afpd/desktop.c | 4 +- etc/afpd/fork.c | 235 ++++++++++++++-------------------- include/atalk/dsi.h | 4 +- include/atalk/globals.h | 1 + libatalk/dsi/dsi_read.c | 3 +- libatalk/dsi/dsi_stream.c | 176 ++++++++++++++++--------- libatalk/util/netatalk_conf.c | 2 + macros/netatalk.m4 | 1 + 8 files changed, 228 insertions(+), 198 deletions(-) diff --git a/etc/afpd/desktop.c b/etc/afpd/desktop.c index 60191503..873cc0b1 100644 --- a/etc/afpd/desktop.c +++ b/etc/afpd/desktop.c @@ -485,15 +485,17 @@ int afp_geticon(AFPObj *obj, char *ibuf, size_t ibuflen _U_, char *rbuf, size_t return AFPERR_PARAM; } +#ifndef WITH_SENDFILE if ((buflen = dsi_readinit(dsi, rbuf, buflen, rc, AFP_OK)) < 0) goto geticon_exit; +#endif *rbuflen = buflen; /* do to the streaming nature, we have to exit if we encounter * a problem. much confusion results otherwise. */ while (*rbuflen > 0) { #ifdef WITH_SENDFILE - if (dsi_stream_read_file(dsi, si.sdt_fd, offset, dsi->datasize) < 0) { + if (dsi_stream_read_file(dsi, si.sdt_fd, offset, dsi->datasize, AFP_OK) < 0) { switch (errno) { case ENOSYS: case EINVAL: /* there's no guarantee that all fs support sendfile */ diff --git a/etc/afpd/fork.c b/etc/afpd/fork.c index 96ac3936..38acf64b 100644 --- a/etc/afpd/fork.c +++ b/etc/afpd/fork.c @@ -739,10 +739,18 @@ int afp_bytelock_ext(AFPObj *obj, char *ibuf, size_t ibuflen, char *rbuf, size_t #undef UNLOCKBIT -static ssize_t read_file(struct ofork *ofork, int eid, - off_t offset, u_char nlmask, - u_char nlchar, char *rbuf, - size_t *rbuflen) +/*! + * Read *rbuflen bytes from fork at offset + * + * @param ofork (r) fork handle + * @param eid (r) data fork or ressource fork entry id + * @param offset (r) offset + * @param rbuf (r) data buffer + * @param rbuflen (rw) in: number of bytes to read, out: bytes read + * + * @return AFP status code + */ +static int read_file(const struct ofork *ofork, int eid, off_t offset, char *rbuf, size_t *rbuflen) { ssize_t cc; int eof = 0; @@ -754,53 +762,23 @@ static ssize_t read_file(struct ofork *ofork, int eid, *rbuflen = 0; return( AFPERR_PARAM ); } - if ( (size_t)cc < *rbuflen ) { - eof = 1; - } - - /* - * Do Newline check. - */ - if ( nlmask != 0 ) { - for ( p = rbuf, q = p + cc; p < q; ) { - if (( *p++ & nlmask ) == nlchar ) { - break; - } - } - if ( p != q ) { - cc = p - rbuf; - eof = 0; - } - } - *rbuflen = cc; - if ( eof ) { - return( AFPERR_EOF ); - } + + if ((size_t)cc < *rbuflen) + return AFPERR_EOF; return AFP_OK; } -/* ----------------------------- - * with ddp, afp_read can return fewer bytes than in reqcount - * so return EOF only if read actually past end of file not - * if offset +reqcount > size of file - * e.g.: - * getfork size ==> 10430 - * read fork offset 0 size 10752 ???? ==> 4264 bytes (without EOF) - * read fork offset 4264 size 6128 ==> 4264 (without EOF) - * read fork offset 9248 size 1508 ==> 1182 (EOF) - * 10752 is a bug in Mac 7.5.x finder - * - * with dsi, should we check that reqcount < server quantum? - */ static int read_fork(AFPObj *obj, char *ibuf, size_t ibuflen _U_, char *rbuf, size_t *rbuflen, int is64) { - struct ofork *ofork; - off_t offset, saveoff, reqcount, savereqcount; - ssize_t cc, err; - int eid; - uint16_t ofrefnum; - u_char nlmask, nlchar; + DSI *dsi = obj->dsi; + struct ofork *ofork; + off_t offset, saveoff, reqcount, savereqcount, size; + ssize_t cc, err; + int eid; + uint16_t ofrefnum; + + /* we break the AFP spec here by not supporting nlmask and nlchar anymore */ ibuf += 2; memcpy(&ofrefnum, ibuf, sizeof( ofrefnum )); @@ -816,37 +794,44 @@ static int read_fork(AFPObj *obj, char *ibuf, size_t ibuflen _U_, char *rbuf, si err = AFPERR_ACCESS; goto afp_read_err; } + + if ( ofork->of_flags & AFPFORK_DATA) { + eid = ADEID_DFORK; + } else if (ofork->of_flags & AFPFORK_RSRC) { + eid = ADEID_RFORK; + } else { /* fork wasn't opened. this should never really happen. */ + err = AFPERR_ACCESS; + goto afp_read_err; + } + offset = get_off_t(&ibuf, is64); reqcount = get_off_t(&ibuf, is64); + /* reqcount isn't always truthful. we need to deal with that. */ + size = ad_size(ofork->of_ad, eid); + LOG(log_debug, logtype_afpd, - "afp_read(off: %" PRIu64 ", size: %" PRIu64 ", fork: %s)", offset, reqcount, - (ofork->of_flags & AFPFORK_DATA) ? "d" : "r"); + "afp_read(off: %" PRIu64 ", len: %" PRIu64 ", fork: %s, size: %" PRIu64 ")", + offset, reqcount, (ofork->of_flags & AFPFORK_DATA) ? "d" : "r", size); - if (is64) { - nlmask = nlchar = 0; + /* subtract off the offset */ + if (reqcount + offset > size) { + reqcount = size - offset; + err = AFPERR_EOF; } - else { - nlmask = *ibuf++; - nlchar = *ibuf++; - } - /* if we wanted to be picky, we could add in the following - * bit: if (afp_version == 11 && !(nlmask == 0xFF || !nlmask)) - */ + + savereqcount = reqcount; + saveoff = offset; + + LOG(log_debug, logtype_afpd, + "afp_read(off: %" PRIu64 ", len: %" PRIu64 ", fork: %s)", + offset, reqcount, (ofork->of_flags & AFPFORK_DATA) ? "d" : "r"); + if (reqcount < 0 || offset < 0) { err = AFPERR_PARAM; goto afp_read_err; } - if ( ofork->of_flags & AFPFORK_DATA) { - eid = ADEID_DFORK; - } else if (ofork->of_flags & AFPFORK_RSRC) { - eid = ADEID_RFORK; - } else { /* fork wasn't opened. this should never really happen. */ - err = AFPERR_ACCESS; - goto afp_read_err; - } - /* zero request count */ err = AFP_OK; if (!reqcount) { @@ -856,94 +841,72 @@ static int read_fork(AFPObj *obj, char *ibuf, size_t ibuflen _U_, char *rbuf, si LOG(log_debug, logtype_afpd, "afp_read(name: \"%s\", offset: %jd, reqcount: %jd)", of_name(ofork), (intmax_t)offset, (intmax_t)reqcount); - savereqcount = reqcount; - saveoff = offset; - if (ad_tmplock(ofork->of_ad, eid, ADLOCK_RD, saveoff, savereqcount,ofork->of_refnum) < 0) { - err = AFPERR_LOCK; - goto afp_read_err; + if (obj->options.flags & OPTION_AFP_READ_LOCK) { + if (ad_tmplock(ofork->of_ad, eid, ADLOCK_RD, offset, reqcount, ofork->of_refnum) < 0) { + err = AFPERR_LOCK; + goto afp_read_err; + } + } + +#ifdef WITH_SENDFILE + if (!(obj->options.flags & OPTION_NOSENDFILE)) { + int fd = ad_readfile_init(ofork->of_ad, eid, &offset, 0); + if (dsi_stream_read_file(dsi, fd, offset, reqcount, err) < 0) { + LOG(log_error, logtype_afpd, "afp_read(%s): ad_readfile: %s", + of_name(ofork), strerror(errno)); + goto afp_read_exit; + } + dsi_readdone(dsi); + goto afp_read_done; } +#endif *rbuflen = MIN(reqcount, *rbuflen); - LOG(log_debug, logtype_afpd, "afp_read(name: \"%s\", offset: %jd, reqcount: %jd): reading %jd bytes from file", - of_name(ofork), (intmax_t)offset, (intmax_t)reqcount, (intmax_t)*rbuflen); - err = read_file(ofork, eid, offset, nlmask, nlchar, rbuf, rbuflen); + err = read_file(ofork, eid, offset, rbuf, rbuflen); if (err < 0) goto afp_read_done; - LOG(log_debug, logtype_afpd, "afp_read(name: \"%s\", offset: %jd, reqcount: %jd): got %jd bytes from file", + + LOG(log_debug, logtype_afpd, + "afp_read(name: \"%s\", offset: %jd, reqcount: %jd): got %jd bytes from file", of_name(ofork), (intmax_t)offset, (intmax_t)reqcount, (intmax_t)*rbuflen); - /* dsi can stream requests. we can only do this if we're not checking - * for an end-of-line character. oh well. */ - if ((*rbuflen < reqcount) && !nlmask) { - DSI *dsi = obj->dsi; - off_t size; + offset += *rbuflen; - /* reqcount isn't always truthful. we need to deal with that. */ - size = ad_size(ofork->of_ad, eid); + /* + * dsi_readinit() returns size of next read buffer. by this point, + * we know that we're sending some data. if we fail, something + * horrible happened. + */ + if ((cc = dsi_readinit(dsi, rbuf, *rbuflen, reqcount, err)) < 0) + goto afp_read_exit; + *rbuflen = cc; - /* subtract off the offset */ - size -= offset; - if (reqcount > size) { - reqcount = size; - err = AFPERR_EOF; - } + while (*rbuflen > 0) { + cc = read_file(ofork, eid, offset, rbuf, rbuflen); + if (cc < 0) + goto afp_read_exit; offset += *rbuflen; - - /* dsi_readinit() returns size of next read buffer. by this point, - * we know that we're sending some data. if we fail, something - * horrible happened. */ - if ((cc = dsi_readinit(dsi, rbuf, *rbuflen, reqcount, err)) < 0) + /* dsi_read() also returns buffer size of next allocation */ + cc = dsi_read(dsi, rbuf, *rbuflen); /* send it off */ + if (cc < 0) goto afp_read_exit; *rbuflen = cc; - /* due to the nature of afp packets, we have to exit if we get - an error. we can't do this with translation on. */ -#ifdef WITH_SENDFILE - if (!(obj->options.flags & OPTION_NOSENDFILE)) { - int fd = ad_readfile_init(ofork->of_ad, eid, &offset, 0); - if (dsi_stream_read_file(dsi, fd, offset, dsi->datasize) < 0) { - switch (errno) { - case EINVAL: - case ENOSYS: - goto afp_read_loop; - default: - LOG(log_error, logtype_afpd, "afp_read(%s): ad_readfile: %s", of_name(ofork), strerror(errno)); - goto afp_read_exit; - } - } - - dsi_readdone(dsi); - goto afp_read_done; - } - afp_read_loop: -#endif - - /* fill up our buffer. */ - while (*rbuflen > 0) { - cc = read_file(ofork, eid, offset, nlmask, nlchar, rbuf,rbuflen); - if (cc < 0) - goto afp_read_exit; - - offset += *rbuflen; - /* dsi_read() also returns buffer size of next allocation */ - cc = dsi_read(dsi, rbuf, *rbuflen); /* send it off */ - if (cc < 0) - goto afp_read_exit; - *rbuflen = cc; - } - dsi_readdone(dsi); - goto afp_read_done; - - afp_read_exit: - LOG(log_error, logtype_afpd, "afp_read(%s): %s", of_name(ofork), strerror(errno)); - dsi_readdone(dsi); - ad_tmplock(ofork->of_ad, eid, ADLOCK_CLR, saveoff, savereqcount,ofork->of_refnum); - obj->exit(EXITERR_CLNT); } + dsi_readdone(dsi); + goto afp_read_done; + +afp_read_exit: + LOG(log_error, logtype_afpd, "afp_read(%s): %s", of_name(ofork), strerror(errno)); + dsi_readdone(dsi); + if (obj->options.flags & OPTION_AFP_READ_LOCK) + ad_tmplock(ofork->of_ad, eid, ADLOCK_CLR, saveoff, savereqcount, ofork->of_refnum); + obj->exit(EXITERR_CLNT); afp_read_done: - ad_tmplock(ofork->of_ad, eid, ADLOCK_CLR, saveoff, savereqcount,ofork->of_refnum); + if (obj->options.flags & OPTION_AFP_READ_LOCK) + ad_tmplock(ofork->of_ad, eid, ADLOCK_CLR, saveoff, savereqcount, ofork->of_refnum); return err; afp_read_err: diff --git a/include/atalk/dsi.h b/include/atalk/dsi.h index 5af4cac5..60afc186 100644 --- a/include/atalk/dsi.h +++ b/include/atalk/dsi.h @@ -174,6 +174,8 @@ extern void dsi_getstatus (DSI *); extern void dsi_close (DSI *); #define DSI_NOWAIT 1 +#define DSI_MSG_MORE 2 + /* low-level stream commands -- in dsi_stream.c */ extern ssize_t dsi_stream_write (DSI *, void *, const size_t, const int mode); extern size_t dsi_stream_read (DSI *, void *, const size_t); @@ -182,7 +184,7 @@ extern int dsi_stream_receive (DSI *); extern int dsi_disconnect(DSI *dsi); #ifdef WITH_SENDFILE -extern ssize_t dsi_stream_read_file(DSI *, int, off_t off, const size_t len); +extern ssize_t dsi_stream_read_file(DSI *, int, off_t off, const size_t len, const int err); #endif /* client writes -- dsi_write.c */ diff --git a/include/atalk/globals.h b/include/atalk/globals.h index e25efdbd..124c3988 100644 --- a/include/atalk/globals.h +++ b/include/atalk/globals.h @@ -39,6 +39,7 @@ #define OPTION_SERVERNOTIF (1 << 2) #define OPTION_NOSENDFILE (1 << 3) #define OPTION_CUSTOMICON (1 << 4) +#define OPTION_AFP_READ_LOCK (1 << 5) /* whether to do AFP spec conforming read locks (default: no) */ #define OPTION_ANNOUNCESSH (1 << 6) #define OPTION_UUID (1 << 7) #define OPTION_ACL2MACCESS (1 << 8) diff --git a/libatalk/dsi/dsi_read.c b/libatalk/dsi/dsi_read.c index 75dfe957..a0cbd872 100644 --- a/libatalk/dsi/dsi_read.c +++ b/libatalk/dsi/dsi_read.c @@ -23,8 +23,7 @@ * it will send off the header plus whatever is in its command * buffer. it returns the amount of stuff still to be read * (constrained by the buffer size). */ -ssize_t dsi_readinit(DSI *dsi, void *buf, const size_t buflen, - const size_t size, const int err) +ssize_t dsi_readinit(DSI *dsi, void *buf, const size_t buflen, const size_t size, const int err) { LOG(log_maxdebug, logtype_dsi, "dsi_readinit: sending %zd bytes from buffer, total size: %zd", buflen, size); diff --git a/libatalk/dsi/dsi_stream.c b/libatalk/dsi/dsi_stream.c index 6cda4345..37d4c121 100644 --- a/libatalk/dsi/dsi_stream.c +++ b/libatalk/dsi/dsi_stream.c @@ -15,17 +15,17 @@ #include #include - -#ifdef HAVE_UNISTD_H #include -#endif - #include #include #include #include #include +#ifdef HAVE_SENDFILEV +#include +#endif + #include #include #include @@ -263,7 +263,7 @@ ssize_t dsi_stream_write(DSI *dsi, void *data, const size_t length, int mode) { size_t written; ssize_t len; - unsigned int flags = 0; + unsigned int flags; dsi->in_write++; written = 0; @@ -273,6 +273,11 @@ ssize_t dsi_stream_write(DSI *dsi, void *data, const size_t length, int mode) if (dsi->flags & DSI_DISCONNECTED) return -1; + if (mode & DSI_MSG_MORE) + flags = MSG_MORE; + else + flags = 0; + while (written < length) { len = send(dsi->socket, (uint8_t *) data + written, length - written, flags); if (len >= 0) { @@ -314,72 +319,127 @@ exit: return written; } +/* Pack a DSI header in wire format */ +static void dsi_header_pack_reply(const DSI *dsi, char *buf) +{ + buf[0] = dsi->header.dsi_flags; + buf[1] = dsi->header.dsi_command; + memcpy(buf + 2, &dsi->header.dsi_requestID, sizeof(dsi->header.dsi_requestID)); + memcpy(buf + 4, &dsi->header.dsi_code, sizeof(dsi->header.dsi_code)); + memcpy(buf + 8, &dsi->header.dsi_len, sizeof(dsi->header.dsi_len)); + memcpy(buf + 12, &dsi->header.dsi_reserved, sizeof(dsi->header.dsi_reserved)); +} + /* --------------------------------- */ #ifdef WITH_SENDFILE -ssize_t dsi_stream_read_file(DSI *dsi, int fromfd, off_t offset, const size_t length) +ssize_t dsi_stream_read_file(DSI *dsi, const int fromfd, off_t offset, const size_t length, const int err) { - int ret = 0; - size_t written; - ssize_t len; - off_t pos = offset; + int ret = 0; + size_t written = 0; + size_t total = length; + ssize_t len; + off_t pos = offset; + char block[DSI_BLOCKSIZ]; +#ifdef HAVE_SENDFILEV + int sfvcnt; + struct sendfilevec vec[2]; + ssize_t nwritten; +#endif - LOG(log_maxdebug, logtype_dsi, "dsi_stream_read_file(send %zd bytes): START", length); + LOG(log_maxdebug, logtype_dsi, "dsi_stream_read_file(off: %jd, len: %zu)", (intmax_t)offset, length); - if (dsi->flags & DSI_DISCONNECTED) - return -1; + if (dsi->flags & DSI_DISCONNECTED) + return -1; - dsi->in_write++; - written = 0; + dsi->in_write++; + + dsi->flags |= DSI_NOREPLY; + dsi->header.dsi_flags = DSIFL_REPLY; + dsi->header.dsi_len = htonl(length); + dsi->header.dsi_code = htonl(err); + dsi_header_pack_reply(dsi, block); + +#ifdef HAVE_SENDFILEV + total += DSI_BLOCKSIZ; + sfvcnt = 2; + vec[0].sfv_fd = SFV_FD_SELF; + vec[0].sfv_flag = 0; + vec[0].sfv_off = block; + vec[0].sfv_len = DSI_BLOCKSIZ; + vec[1].sfv_fd = fromfd; + vec[1].sfv_flag = 0; + vec[1].sfv_off = offset; + vec[1].sfv_len = length; +#else + dsi_stream_write(dsi, block, sizeof(block), DSI_MSG_MORE); +#endif - while (written < length) { - len = sys_sendfile(dsi->socket, fromfd, &pos, length - written); - - if (len < 0) { - if (errno == EINTR) - continue; - if (errno == EINVAL || errno == ENOSYS) { - ret = -1; - goto exit; - } - if (errno == EAGAIN || errno == EWOULDBLOCK) { -#if defined(SOLARIS) || defined(FREEBSD) - if (pos > offset) { - /* we actually have sent sth., adjust counters and keep trying */ - len = pos - offset; - written += len; - offset = pos; - } + while (written < total) { +#ifdef HAVE_SENDFILEV + nwritten = 0; + len = sendfilev(dsi->socket, vec, sfvcnt, &nwritten); +#else + len = sys_sendfile(dsi->socket, fromfd, &pos, total - written); #endif - if (dsi_peek(dsi)) { - /* can't go back to blocking mode, exit, the next read - will return with an error and afpd will die. - */ - break; - } - continue; - } - LOG(log_error, logtype_dsi, "dsi_stream_read_file: %s", strerror(errno)); - break; - } - else if (!len) { - /* afpd is going to exit */ - ret = -1; - goto exit; - } - else - written += len; - } + if (len < 0) { + switch (errno) { + case EINTR: + case EAGAIN: +#if defined(SOLARIS) || defined(FREEBSD) +#ifdef HAVE_SENDFILEV + len = (size_t)nwritten; +#else + if (pos > offset) { + /* we actually have sent sth., adjust counters and keep trying */ + len = pos - offset; + written += len; + offset = pos; + } +#endif /* HAVE_SENDFILEV */ +#endif /* defined(SOLARIS) || defined(FREEBSD) */ + if (dsi_peek(dsi)) { + ret = -1; + goto exit; + } + break; + default: + LOG(log_error, logtype_dsi, "dsi_stream_read_file: %s", strerror(errno)); + ret = -1; + goto exit; + } + } else if (len == 0) { + /* afpd is going to exit */ + ret = -1; + goto exit; + } +#ifdef HAVE_SENDFILEV + if (sfvcnt == 2 && len >= vec[0].sfv_len) { + vec[1].sfv_off += len - vec[0].sfv_len; + vec[1].sfv_len -= len - vec[0].sfv_len; - dsi->write_count += written; + vec[0] = vec[1]; + sfvcnt = 1; + } else { + vec[0].sfv_off += len; + vec[0].sfv_len -= len; + } +#endif /* HAVE_SENDFILEV */ + + written += len; + } +#ifdef HAVE_SENDFILEV + written -= DSI_BLOCKSIZ; +#endif + dsi->write_count += written; exit: - dsi->in_write--; - LOG(log_maxdebug, logtype_dsi, "dsi_stream_read_file: sent: %zd", written); - if (ret != 0) - return -1; - return written; + dsi->in_write--; + LOG(log_maxdebug, logtype_dsi, "dsi_stream_read_file: written: %zd", written); + if (ret != 0) + return -1; + return written; } #endif diff --git a/libatalk/util/netatalk_conf.c b/libatalk/util/netatalk_conf.c index cd2246ed..39a725c2 100644 --- a/libatalk/util/netatalk_conf.c +++ b/libatalk/util/netatalk_conf.c @@ -1448,6 +1448,8 @@ int afp_config_parse(AFPObj *AFPObj, char *processname) options->flags |= OPTION_NOSENDFILE; if (iniparser_getboolean(config, INISEC_GLOBAL, "solaris share reservations", 1)) options->flags |= OPTION_SHARE_RESERV; + if (iniparser_getboolean(config, INISEC_GLOBAL, "afp read locks", 0)) + options->flags |= OPTION_AFP_READ_LOCK; if (!iniparser_getboolean(config, INISEC_GLOBAL, "save password", 1)) options->passwdbits |= PASSWD_NOSAVE; if (iniparser_getboolean(config, INISEC_GLOBAL, "set password", 0)) diff --git a/macros/netatalk.m4 b/macros/netatalk.m4 index ce587f08..78b708d5 100644 --- a/macros/netatalk.m4 +++ b/macros/netatalk.m4 @@ -918,6 +918,7 @@ if test x"$netatalk_cv_search_sendfile" = x"yes"; then AC_DEFINE(SENDFILE_FLAVOR_SOLARIS, 1, [Solaris sendfile()]) AC_SEARCH_LIBS(sendfile, sendfile) AC_CHECK_FUNC([sendfile], [netatalk_cv_HAVE_SENDFILE=yes]) + AC_CHECK_FUNCS([sendfilev]) ;; *freebsd*) -- 2.39.2