]> arthur.barton.de Git - netatalk.git/commitdiff
Remove SO_RCVTIMEO/SO_SNDTIMEO, switch to non-blocking IO
authorFrank Lahm <franklahm@googlemail.com>
Wed, 13 Oct 2010 19:21:40 +0000 (21:21 +0200)
committerFrank Lahm <franklahm@googlemail.com>
Wed, 13 Oct 2010 19:21:40 +0000 (21:21 +0200)
NEWS
etc/cnid_dbd/cnid_metad.c
etc/cnid_dbd/comm.c
etc/cnid_dbd/comm.h
etc/cnid_dbd/usockfd.c
libatalk/cnid/dbd/cnid_dbd.c

diff --git a/NEWS b/NEWS
index a962a4ccfd2678ca1755a7d0ddb9112a75a94c48..4aae1d9558c5a5f1515af1c7f0dd98ab83cfc9f4 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -2,7 +2,7 @@ Changes in 2.2
 ==============
 
 * UPD: AppleTalk ist disabled by default at configuration time. If needed
-       use --enable-ddp.
+       use configure switch --enable-ddp.
 * NEW: ad utility: ad cp
 * NEW: ad utility: ad rm
 * NEW: ad utility: ad mv
@@ -10,6 +10,10 @@ Changes in 2.2
 * NEW: afpd: POSIX 1e ACL support
 * NEW: afpd: automagic Zeroconf registration with avahi, registering both
        the service _afpovertcp._tcp and TimeMachine volumes with _adisk._tcp.
+* FIX: afpd: Solaris 10 compatibilty fix: don't use SO_SNDTIMEO/SO_RCVTIMEO,
+       use non-blocking IO and select instead.
+* FIX: cnid_dbd: Solaris 10 compatibilty fix: don't use SO_SNDTIMEO/SO_RCVTIMEO,
+       use non-blocking IO and select instead.
 
 Changes in 2.1.4
 ================
index 4b089ae8a5b6625349efaf61464339eed63c4503..bdeb8898978901b7b4f04ce6b39640fcd02416ae 100644 (file)
@@ -1,9 +1,8 @@
 /*
- * $Id: cnid_metad.c,v 1.22 2009-11-16 02:04:47 didg Exp $
- *
  * Copyright (C) Joerg Lenneis 2003
- * All Rights Reserved.  See COPYING.
+ * Copyright (C) Frank Lahm 2009, 2010
  *
+ * All Rights Reserved.  See COPYING.
  */
 
 /* 
@@ -22,6 +21,8 @@
    Result:
                        via TCP socket
    4.       afpd          ------->         cnid_dbd
+
+   cnid_metad and cnid_dbd have been converted to non-blocking IO in 2010.
  */
 
 
 #include <errno.h>
 #include <string.h>
 #include <signal.h>
-#ifdef HAVE_SYS_TYPES_H
 #include <sys/types.h>
-#endif
-#ifdef HAVE_SYS_TIME_H
 #include <sys/time.h>
-#endif
-#ifdef HAVE_SYS_WAIT_H
 #include <sys/wait.h>
-#endif
-#ifdef HAVE_SYS_UIO_H
 #include <sys/uio.h>
-#endif
 #include <sys/un.h>
-#define _XPG4_2 1
 #include <sys/socket.h>
 #include <stdio.h>
 #include <time.h>
@@ -604,8 +596,13 @@ int main(int argc, char *argv[])
         if (rqstfd <= 0)
             continue;
 
-        /* TODO: Check out read errors, broken pipe etc. in libatalk. Is
-           SIGIPE ignored there? Answer: Ignored for dsi, but not for asp ... */
+        /*
+         * Note:
+         * although we're in non-blocking mode, I didn't bother adding EAGAIN/select
+         * loops around read, because we're really reading only very small amounts here,
+         * and as we've just got the connection assuming the sent bytes are
+         * available for reading seems reasonable.
+         */
         ret = read(rqstfd, &len, sizeof(int));
         if (!ret) {
             /* already close */
index 0dae7be74f13ded2e1043f975422dafcb1303690..03827b4ec0ce355d48642239e67dff56531e8228 100644 (file)
@@ -1,7 +1,7 @@
 /*
- * $Id: comm.c,v 1.6 2009-10-19 08:09:07 didg Exp $
- *
  * Copyright (C) Joerg Lenneis 2003
+ * Copyright (C) Frank Lahm 2010
+ *
  * All Rights Reserved.  See COPYING.
  */
 
 #include <stdlib.h>
 #include <string.h>
 #include <errno.h>
-
-#ifdef HAVE_UNISTD_H
 #include <unistd.h>
-#endif
-
 #include <sys/param.h>
-#define _XPG4_2 1
-#include <sys/socket.h>
-
-#ifdef HAVE_SYS_TYPES_H
 #include <sys/types.h>
-#endif
-
-#ifdef HAVE_SYS_TIME_H
 #include <sys/time.h>
-#endif
-
-#ifdef HAVE_SYS_UIO_H
 #include <sys/uio.h>
-#endif
-
-#ifdef HAVE_SYS_SOCKET_H
 #include <sys/socket.h>
-#endif
-
 #include <sys/select.h>
-
 #include <assert.h>
 #include <time.h>
 
 #include <atalk/logger.h>
+#include <atalk/util.h>
 #include <atalk/cnid_dbd_private.h>
 
 #include "db_param.h"
@@ -87,6 +68,62 @@ static void invalidate_fd(int fd)
     return;
 }
 
+/*!
+ * non-blocking drop-in replacement for read with timeout using select
+ *
+ * @param socket   (r)  must be nonblocking !
+ * @param data     (rw) buffer for the read data
+ * @param lenght   (r)  how many bytes to read
+ * @param timeout  (r)  number of seconds to try reading
+ *
+ * @returns number of bytes actually read or -1 on fatal error
+ */
+static ssize_t readt(int socket, void *data, const size_t length, int timeout)
+{
+    size_t stored;
+    ssize_t len;
+    struct timeval tv;
+    fd_set rfds;
+    int ret;
+
+    stored = 0;
+
+    while (stored < length) {
+        len = read(socket, (u_int8_t *) data + stored, length - stored);
+        if (len == -1) {
+            switch (errno) {
+            case EINTR:
+                continue;
+            case EAGAIN:
+                tv.tv_usec = 0;
+                tv.tv_sec  = timeout;
+
+                FD_ZERO(&rfds);
+                FD_SET(socket, &rfds);
+                while ((ret = select(socket + 1, &rfds, NULL, NULL, &tv)) < 1) {
+                    switch (ret) {
+                    case 0:
+                        LOG(log_warning, logtype_cnid, "select timeout 1s");
+                        return stored;
+                    default: /* -1 */
+                        LOG(log_error, logtype_cnid, "select: %s", strerror(errno));
+                        return -1;
+                    }
+                }
+                continue;
+            }
+            LOG(log_error, logtype_cnid, "read: %s", strerror(errno));
+            return -1;
+        }
+        else if (len > 0)
+            stored += len;
+        else
+            break;
+    }
+    return stored;
+}
+
+
 static int recv_cred(int fd)
 {
     int ret;
@@ -267,8 +304,14 @@ int comm_rcv(struct cnid_dbd_rqst *rqst, time_t timeout, const sigset_t *sigmask
 
     LOG(log_maxdebug, logtype_cnid, "comm_rcv: got data on fd %u", cur_fd);
 
+    if (setnonblock(cur_fd, 1) != 0) {
+        LOG(log_error, logtype_cnid, "comm_rcv: setnonblock: %s", strerror(errno));
+        return -1;
+    }
+
     nametmp = rqst->name;
-    if ((b = read(cur_fd, rqst, sizeof(struct cnid_dbd_rqst))) != sizeof(struct cnid_dbd_rqst)) {
+    if ((b = readt(cur_fd, rqst, sizeof(struct cnid_dbd_rqst), CNID_DBD_TIMEOUT))
+        != sizeof(struct cnid_dbd_rqst)) {
         if (b)
             LOG(log_error, logtype_cnid, "error reading message header: %s", strerror(errno));
         invalidate_fd(cur_fd);
@@ -276,7 +319,8 @@ int comm_rcv(struct cnid_dbd_rqst *rqst, time_t timeout, const sigset_t *sigmask
         return 0;
     }
     rqst->name = nametmp;
-    if (rqst->namelen && read(cur_fd, rqst->name, rqst->namelen) != rqst->namelen) {
+    if (rqst->namelen && readt(cur_fd, rqst->name, rqst->namelen, CNID_DBD_TIMEOUT)
+        != rqst->namelen) {
         LOG(log_error, logtype_cnid, "error reading message name: %s", strerror(errno));
         invalidate_fd(cur_fd);
         return 0;
index 614bd27da6adbeedbe166768f08fe811f6c49d7a..46d91fc18fab1c8e97c815e91393ea73c418426e 100644 (file)
@@ -1,6 +1,4 @@
 /*
- * $Id: comm.h,v 1.5 2009-10-19 08:09:07 didg Exp $
- *
  * Copyright (C) Joerg Lenneis 2003
  * All Rights Reserved.  See COPYING.
  */
@@ -8,6 +6,8 @@
 #ifndef CNID_DBD_COMM_H
 #define CNID_DBD_COMM_H 1
 
+/* number of seconds to try reading in readt */
+#define CNID_DBD_TIMEOUT 1
 
 #include <atalk/cnid_dbd_private.h>
 
index 6c97fe73b56ea1925402720c9988523378b3fcf3..30bb9e705896b610841bd71432fd0d0a6c5c5161 100644 (file)
@@ -1,6 +1,4 @@
 /*
- * $Id: usockfd.c,v 1.6 2009-11-05 14:38:07 franklahm Exp $
- *
  * Copyright (C) Joerg Lenneis 2003
  * All Rights Reserved.  See COPYING.
  */
@@ -176,11 +174,8 @@ int usockfd_check(int sockfd, const sigset_t *sigset)
                 strerror(errno));
             return -1;
         }
-        tv.tv_sec = 5;
-        tv.tv_usec = 0;
-        if (setsockopt(fd, SOL_SOCKET, SO_RCVTIMEO, &tv, sizeof(tv)) < 0) {
-            LOG(log_error, logtype_cnid, "set SO_RCVTIMEO: %s", strerror(errno));
-            close(fd);
+        if (setnonblock(fd, 1) != 0) {
+            LOG(log_error, logtype_cnid, "setnonblock: %s", strerror(errno));
             return -1;
         }
         return fd;
index c98adafcd152f872d24904bf0a69c97112afb9e0..47049d0ff9ec20b9a49d0b4c2a4c3bd99b99a445 100644 (file)
@@ -1,6 +1,4 @@
 /*
- * $Id: cnid_dbd.c,v 1.17 2010/03/31 09:47:32 franklahm Exp $
- *
  * Copyright (C) Joerg Lenneis 2003
  * All Rights Reserved.  See COPYING.
  */
 #include <atalk/logger.h>
 #include <atalk/adouble.h>
 #include <atalk/cnid.h>
-#include "cnid_dbd.h"
 #include <atalk/cnid_dbd_private.h>
+#include <atalk/util.h>
+
+#include "cnid_dbd.h"
 
 #ifndef SOL_TCP
 #define SOL_TCP IPPROTO_TCP
 #endif /* ! SOL_TCP */
 
+/* Wait MAX_DELAY seconds before a request to the CNID server times out */
+#define MAX_DELAY 10
+
 static void RQST_RESET(struct cnid_dbd_rqst  *r)
 {
     memset(r, 0, sizeof(struct cnid_dbd_rqst ));
 }
 
-/* ----------- */
-#define MAX_DELAY 10
-
-/* *MUST* be < afp tickle or it's never triggered (got EINTR first) */
-#define SOCK_DELAY 11
-
 static void delay(int sec)
 {
     struct timeval tv;
@@ -69,7 +66,6 @@ static void delay(int sec)
 static int tsock_getfd(const char *host, const char *port)
 {
     int sock = -1;
-    struct timeval tv;
     int attr;
     int err;
     struct addrinfo hints, *servinfo, *p;
@@ -81,46 +77,68 @@ static int tsock_getfd(const char *host, const char *port)
     hints.ai_flags = AI_NUMERICSERV;
 
     if ((err = getaddrinfo(host, port, &hints, &servinfo)) != 0) {
-        LOG(log_error, logtype_default, "tsock_getfd: getaddrinfo: CNID server %s:%s : %s\n", host, port, gai_strerror(err));
+        LOG(log_error, logtype_default, "tsock_getfd: getaddrinfo: CNID server %s:%s : %s\n",
+            host, port, gai_strerror(err));
         return -1;
     }
 
     /* loop through all the results and bind to the first we can */
     for (p = servinfo; p != NULL; p = p->ai_next) {
         if ((sock = socket(p->ai_family, p->ai_socktype, p->ai_protocol)) == -1) {
-            LOG(log_info, logtype_default, "tsock_getfd: socket CNID server %s:: %s", host, strerror(errno));
+            LOG(log_info, logtype_default, "tsock_getfd: socket CNID server %s:: %s",
+                host, strerror(errno));
                 continue;
         }
 
         attr = 1;
         if (setsockopt(sock, SOL_TCP, TCP_NODELAY, &attr, sizeof(attr)) == -1) {
-            LOG(log_error, logtype_cnid, "getfd: set TCP_NODELAY CNID server %s: %s", host, strerror(errno));
-            close(sock);
-            continue;
-        }
-        
-        tv.tv_sec = SOCK_DELAY;
-        tv.tv_usec = 0;
-        if (setsockopt(sock, SOL_SOCKET, SO_RCVTIMEO, &tv, sizeof(tv)) < 0) {
-            LOG(log_error, logtype_cnid, "getfd: set SO_RCVTIMEO CNID server %s: %s", host, strerror(errno));
+            LOG(log_error, logtype_cnid, "getfd: set TCP_NODELAY CNID server %s: %s",
+                host, strerror(errno));
             close(sock);
-            continue;
+            sock = -1;
+            return -1;
         }
 
-        tv.tv_sec = SOCK_DELAY;
-        tv.tv_usec = 0;
-        if (setsockopt(sock, SOL_SOCKET, SO_SNDTIMEO, &tv, sizeof(tv)) < 0) {
-            LOG(log_error, logtype_cnid, "getfd: set SO_SNDTIMEO CNID server %s: %s", host, strerror(errno));
+        if (setnonblock(sock, 1) != 0) {
+            LOG(log_error, logtype_cnid, "getfd: setnonblock: %s", strerror(err));
             close(sock);
-            continue;
+            sock = -1;
+            return -1;
         }
         
         if (connect(sock, p->ai_addr, p->ai_addrlen) == -1) {
-            err = errno;
-            close(sock);
-            sock=-1;
-            LOG(log_error, logtype_cnid, "getfd: connect CNID server %s: %s", host, strerror(err));
-            continue;
+            if (errno == EINPROGRESS) {
+                struct timeval tv;
+                tv.tv_usec = 0;
+                tv.tv_sec  = 1; /* give it one second ... */
+                fd_set wfds;
+                FD_ZERO(&wfds);
+                FD_SET(sock, &wfds);
+                if (select(sock + 1, NULL, &wfds, NULL, &tv) < 1) {
+                    /* give up */
+                    LOG(log_error, logtype_cnid, "getfd: select timed out for CNID server %s: %s",
+                        host, strerror(errno));
+                    close(sock);
+                    sock = -1;
+                    continue;
+                }
+                int optval;
+                socklen_t optlen;
+                if (getsockopt(sock, SOL_SOCKET, SO_ERROR, &optval, &optlen) != 0 || optval != 0) {
+                    /* somethings still wrong */
+                    LOG(log_error, logtype_cnid, "getfd: getsockopt error with CNID server %s: %s",
+                        host, strerror(errno));
+                    close(sock);
+                    sock = -1;
+                    continue;
+                }
+            } else {
+                LOG(log_error, logtype_cnid, "getfd: connect CNID server %s: %s",
+                    host, strerror(errno));
+                close(sock);
+                sock = -1;
+                continue;
+            }
         }
         
         /* We've got a socket */
@@ -130,47 +148,47 @@ static int tsock_getfd(const char *host, const char *port)
     freeaddrinfo(servinfo);
 
     if (p == NULL) {
-        LOG(log_error, logtype_cnid, "tsock_getfd: no suitable network config from CNID server %s:%s", host, port);
+        LOG(log_error, logtype_cnid, "tsock_getfd: no suitable network config from CNID server %s:%s",
+            host, port);
         return -1;
     }
 
     return(sock);
 }
 
-/* --------------------- */
-static int write_vec(int fd, struct iovec *iov, size_t towrite)
+/*!
+ * Write "towrite" bytes using writev on non-blocking fd
+ *
+ * Every short write is considered an error, transmit can handle that.
+ *
+ * @param fd      (r) socket fd which must be non-blocking
+ * @param iov     (r) iovec for writev
+ * @param towrite (r) number of bytes in all iovec elements
+ * @param vecs    (r) number of iovecs in array
+ *
+ * @returns "towrite" bytes written or -1 on error
+ */
+static int write_vec(int fd, struct iovec *iov, ssize_t towrite, int vecs)
 {
     ssize_t len;
-    size_t len1;
 
-    len1 =  iov[1].iov_len;
-    while (towrite > 0) {
-        if (((len = writev(fd, iov, 2)) == -1 && errno == EINTR) || !len)
+    while (1) {
+        if (((len = writev(fd, iov, vecs)) == -1 && errno == EINTR))
             continue;
 
-        if ((size_t)len == towrite) /* wrote everything out */
+        if (len == towrite) /* wrote everything out */
             break;
-        else if (len < 0) { /* error */
-            return -1;
-        }
 
-        towrite -= len;
-        if (towrite > len1) { /* skip part of header */
-            iov[0].iov_base = (char *) iov[0].iov_base + len;
-            iov[0].iov_len -= len;
-        } else { /* skip to data */
-            if (iov[0].iov_len) {
-                len -= iov[0].iov_len;
-                iov[0].iov_len = 0;
-            }
-            iov[1].iov_base = (char *) iov[1].iov_base + len;
-            iov[1].iov_len -= len;
-        }
+        if (len == -1)
+            LOG(log_error, logtype_cnid, "write_vec: short write: %s", strerror(errno));
+        else
+            LOG(log_error, logtype_cnid, "write_vec: short write: %d", len);
+        return len;
     }
 
-    LOG(log_maxdebug, logtype_cnid, "write_vec: wrote %d bytes", towrite);
+    LOG(log_maxdebug, logtype_cnid, "write_vec: wrote %d bytes", len);
 
-    return 0;
+    return len;
 }
 
 /* --------------------- */
@@ -194,7 +212,7 @@ static int init_tsock(CNID_private *db)
     iov[1].iov_base = db->db_dir;
     iov[1].iov_len  = len;
 
-    if (write_vec(fd, iov, len + sizeof(int)) < 0) {
+    if (write_vec(fd, iov, len + sizeof(int), 2) != len + sizeof(int)) {
         LOG(log_error, logtype_cnid, "init_tsock: Error/short write: %s", strerror(errno));
         close(fd);
         return -1;
@@ -210,26 +228,21 @@ static int send_packet(CNID_private *db, struct cnid_dbd_rqst *rqst)
 {
     struct iovec iov[2];
     size_t towrite;
-
-    if (!rqst->namelen) {
-        if (write(db->fd, rqst, sizeof(struct cnid_dbd_rqst)) != sizeof(struct cnid_dbd_rqst)) {
-            LOG(log_warning, logtype_cnid, "send_packet: Error/short write rqst (db_dir %s): %s",
-                db->db_dir, strerror(errno));
-            return -1;
-        }
-        LOG(log_maxdebug, logtype_cnid, "send_packet: OK");
-        return 0;
-    }
+    int vecs;
 
     iov[0].iov_base = rqst;
     iov[0].iov_len  = sizeof(struct cnid_dbd_rqst);
+    towrite = sizeof(struct cnid_dbd_rqst);
+    vecs = 1;
 
-    iov[1].iov_base = rqst->name;
-    iov[1].iov_len  = rqst->namelen;
-
-    towrite = sizeof(struct cnid_dbd_rqst) +rqst->namelen;
+    if (rqst->namelen) {
+        iov[1].iov_base = rqst->name;
+        iov[1].iov_len  = rqst->namelen;
+        towrite += rqst->namelen;
+        vecs++;
+    }
 
-    if (write_vec(db->fd, iov, towrite) < 0) {
+    if (write_vec(db->fd, iov, towrite, vecs) != towrite) {
         LOG(log_warning, logtype_cnid, "send_packet: Error writev rqst (db_dir %s): %s",
             db->db_dir, strerror(errno));
         return -1;
@@ -262,18 +275,50 @@ static int dbd_reply_stamp(struct cnid_dbd_rply *rply)
     return 0;
 }
 
-/* ------------------- */
-static ssize_t dbd_read(int socket, void *data, const size_t length)
+/*!
+ * Non-blocking read "length" bytes within 1 second using select
+ *
+ * @param socket   (r)  must be nonblocking !
+ * @param data     (rw) buffer for the read data
+ * @param lenght   (r)  how many bytes to read
+ *
+ * @returns number of bytes actually read or -1 on fatal error
+ */
+static ssize_t read_packet(int socket, void *data, const size_t length)
 {
     size_t stored;
     ssize_t len;
-  
+    struct timeval tv;
+    fd_set rfds;
+    int ret;
+
     stored = 0;
+
     while (stored < length) {
         len = read(socket, (u_int8_t *) data + stored, length - stored);
         if (len == -1) {
-            if (errno == EINTR)
+            switch (errno) {
+            case EINTR:
+                continue;
+            case EAGAIN:
+                tv.tv_usec = 0;
+                tv.tv_sec  = 1;
+
+                FD_ZERO(&rfds);
+                FD_SET(socket, &rfds);
+                while ((ret = select(socket + 1, &rfds, NULL, NULL, &tv)) < 1) {
+                    switch (ret) {
+                    case 0:
+                        LOG(log_warning, logtype_cnid, "select timeout 1s");
+                        return stored;
+                    default: /* -1 */
+                        LOG(log_error, logtype_cnid, "select: %s", strerror(errno));
+                        return -1;
+                    }
+                }
                 continue;
+            }
+            LOG(log_error, logtype_cnid, "read: %s", strerror(errno));
             return -1;
         }
         else if (len > 0)
@@ -301,7 +346,7 @@ static int dbd_rpc(CNID_private *db, struct cnid_dbd_rqst *rqst, struct cnid_dbd
     len = rply->namelen;
     nametmp = rply->name;
 
-    ret = dbd_read(db->fd, rply, sizeof(struct cnid_dbd_rply));
+    ret = read_packet(db->fd, rply, sizeof(struct cnid_dbd_rply));
 
     if (ret != sizeof(struct cnid_dbd_rply)) {
         LOG(log_error, logtype_cnid, "dbd_rpc: Error reading header from fd (db_dir %s): %s",
@@ -316,7 +361,7 @@ static int dbd_rpc(CNID_private *db, struct cnid_dbd_rqst *rqst, struct cnid_dbd
             db->db_dir, rply->name, rply->namelen, len);
         return -1;
     }
-    if (rply->namelen && (ret = dbd_read(db->fd, rply->name, rply->namelen)) != (ssize_t)rply->namelen) {
+    if (rply->namelen && (ret = read_packet(db->fd, rply->name, rply->namelen)) != (ssize_t)rply->namelen) {
         LOG(log_error, logtype_cnid, "dbd_rpc: Error reading name from fd (db_dir %s): %s",
             db->db_dir, ret == -1?strerror(errno):"closed");
         return -1;
@@ -366,8 +411,7 @@ static int transmit(CNID_private *db, struct cnid_dbd_rqst *rqst, struct cnid_db
                     return -1;
                 }
                 LOG(log_debug7, logtype_cnid, "transmit: ... OK.");
-            }
-            else {
+            } else { /* db->notfirst == 0 */
                 db->notfirst = 1;
                 if (db->client_stamp)
                     memcpy(db->client_stamp, stamp, ADEDLEN_PRIVSYN);
@@ -393,7 +437,7 @@ static int transmit(CNID_private *db, struct cnid_dbd_rqst *rqst, struct cnid_db
                 return -1;
             }
             /* sleep a little before retry */
-            delay(2);
+            delay(1);
         } else {
             clean = 0; /* false... next time sleep */
             time(&orig);