From 10a1d059b2ac0f64036ac93525d78ffc359d67aa Mon Sep 17 00:00:00 2001 From: Simon Rettberg Date: Tue, 31 Oct 2017 22:47:04 +0100 Subject: [SERVER] net.c: Refactoring and renaming Less copy & paste for sendfile alternatives, changed naming of old dnbd3_* functions moved over from server.c a while ago. --- src/server/net.c | 91 ++++++++++++++++++++------------------------------------ 1 file changed, 33 insertions(+), 58 deletions(-) diff --git a/src/server/net.c b/src/server/net.c index c11a78a..ee30697 100644 --- a/src/server/net.c +++ b/src/server/net.c @@ -51,9 +51,9 @@ static uint64_t totalBytesSent = 0; static pthread_spinlock_t statisticsSentLock; // Adding and removing clients -- list management -static void dnbd3_removeClient(dnbd3_client_t *client); -static dnbd3_client_t* dnbd3_freeClient(dnbd3_client_t *client); -static bool dnbd3_addClient(dnbd3_client_t *client); +static bool addToList(dnbd3_client_t *client); +static void removeFromList(dnbd3_client_t *client); +static dnbd3_client_t* freeClientStruct(dnbd3_client_t *client); /** * Update global sent stats. Hold client's statsLock when calling. @@ -218,8 +218,8 @@ void* net_handleNewConnection(void *clientPtr) client->lastBytesSent = 0; spin_unlock( &client->statsLock ); - if ( !dnbd3_addClient( client ) ) { - dnbd3_freeClient( client ); + if ( !addToList( client ) ) { + freeClientStruct( client ); logadd( LOG_WARNING, "Could not add new client to list when connecting" ); return NULL; } @@ -418,61 +418,39 @@ void* net_handleNewConnection(void *clientPtr) realBytes = image->realFilesize - offset; } while ( done < realBytes ) { + // TODO: Should we consider EOPNOTSUPP on BSD for sendfile and fallback to read/write? + // Linux would set EINVAL or ENOSYS instead, which it unfortunately also does for a couple of other failures :/ + // read/write would kill performance anyways so a fallback would probably be of little use either way. #ifdef AFL_MODE char buf[1000]; size_t cnt = realBytes - done; if ( cnt > 1000 ) { cnt = 1000; } - const ssize_t ret = pread( image_file, buf, cnt, offset ); - if ( ret <= 0 ) { + const ssize_t sent = pread( image_file, buf, cnt, foffset ); + if ( sent > 0 ) { + //write( client->sock, buf, sent ); // This is not verified in any way, so why even do it... + } else { const int err = errno; - if ( lock ) pthread_mutex_unlock( &client->sendMutex ); - if ( ret == -1 ) { - if ( err != EPIPE && err != ECONNRESET && err != ESHUTDOWN - && err != EAGAIN && err != EWOULDBLOCK ) { - logadd( LOG_DEBUG1, "sendfile to %s failed (image to net. sent %d/%d, errno=%d)", - client->hostName, (int)done, (int)realBytes, err ); - } - if ( err == EBADF || err == EFAULT || err == EINVAL || err == EIO ) { - logadd( LOG_INFO, "Disabling %s:%d", image->name, image->rid ); - image->working = false; - } - } - goto exit_client_cleanup; - } - write( client->sock, buf, ret ); - done += ret; #elif defined(__linux__) - const ssize_t ret = sendfile( client->sock, image_file, &foffset, realBytes - done ); - if ( ret <= 0 ) { + const ssize_t sent = sendfile( client->sock, image_file, &foffset, realBytes - done ); + if ( sent <= 0 ) { const int err = errno; - if ( lock ) pthread_mutex_unlock( &client->sendMutex ); - if ( ret == -1 ) { - if ( err != EPIPE && err != ECONNRESET && err != ESHUTDOWN - && err != EAGAIN && err != EWOULDBLOCK ) { - logadd( LOG_DEBUG1, "sendfile to %s failed (image to net. sent %d/%d, errno=%d)", - client->hostName, (int)done, (int)realBytes, err ); - } - if ( err == EBADF || err == EFAULT || err == EINVAL || err == EIO ) { - logadd( LOG_INFO, "Disabling %s:%d", image->name, image->rid ); - image->working = false; - } - } - goto exit_client_cleanup; - } - done += ret; #elif defined(__FreeBSD__) off_t sent; - int ret = sendfile( image_file, client->sock, foffset, realBytes - done, NULL, &sent, 0 ); - const int err = errno; - if ( ret < 0 ) { - if ( err == EAGAIN ) { - done += sent; - continue; - } + const int ret = sendfile( image_file, client->sock, foffset, realBytes - done, NULL, &sent, 0 ); + if ( ret == -1 || sent == 0 ) { + const int err = errno; if ( ret == -1 ) { - if ( lock ) pthread_mutex_unlock( &client->sendMutex ); + if ( err == EAGAIN || err == EINTR ) { // EBUSY? manpage doesn't explicitly mention *sent here.. But then again we dont set the according flag anyways + done += sent; + continue; + } + sent = -1; + } +#endif + if ( lock ) pthread_mutex_unlock( &client->sendMutex ); + if ( sent == -1 ) { if ( err != EPIPE && err != ECONNRESET && err != ESHUTDOWN && err != EAGAIN && err != EWOULDBLOCK ) { logadd( LOG_DEBUG1, "sendfile to %s failed (image to net. sent %d/%d, errno=%d)", @@ -484,13 +462,10 @@ void* net_handleNewConnection(void *clientPtr) } } goto exit_client_cleanup; - } else { - done += sent; - if ( sent == 0 ) break; } -#endif + done += sent; } - logadd( LOG_DEBUG2, "Send %i to %s", realBytes, client->hostName ); + logadd( LOG_DEBUG2, "Send %i to %s", (int)realBytes, client->hostName ); if ( request.size > (uint32_t)realBytes ) { if ( !sendPadding( client->sock, request.size - (uint32_t)realBytes ) ) { if ( lock ) pthread_mutex_unlock( &client->sendMutex ); @@ -559,9 +534,9 @@ set_name: ; } } exit_client_cleanup: ; - dnbd3_removeClient( client ); + removeFromList( client ); net_updateGlobalSentStatsFromClient( client ); // Don't need client's lock here as it's not active anymore - dnbd3_freeClient( client ); // This will also call image_release on client->image + freeClientStruct( client ); // This will also call image_release on client->image return NULL ; fail_preadd: ; close( client->sock ); @@ -672,7 +647,7 @@ void net_waitForAllDisconnected() * Remove a client from the clients array * Locks on: _clients_lock */ -static void dnbd3_removeClient(dnbd3_client_t *client) +static void removeFromList(dnbd3_client_t *client) { int i; spin_lock( &_clients_lock ); @@ -691,7 +666,7 @@ static void dnbd3_removeClient(dnbd3_client_t *client) * Locks on: _clients[].lock, _images[].lock * might call functions that lock on _images, _image[], uplink.queueLock, client.sendMutex */ -static dnbd3_client_t* dnbd3_freeClient(dnbd3_client_t *client) +static dnbd3_client_t* freeClientStruct(dnbd3_client_t *client) { spin_lock( &client->lock ); pthread_mutex_lock( &client->sendMutex ); @@ -720,7 +695,7 @@ static dnbd3_client_t* dnbd3_freeClient(dnbd3_client_t *client) * Add client to the clients array. * Locks on: _clients_lock */ -static bool dnbd3_addClient(dnbd3_client_t *client) +static bool addToList(dnbd3_client_t *client) { int i; spin_lock( &_clients_lock ); -- cgit v1.2.3-55-g7522