[SCM] Samba Shared Repository - branch master updated

Ralph Böhme slow at samba.org
Tue Oct 24 10:33:02 UTC 2023


The branch, master has been updated
       via  3f4f80edba2 smb2_server: monitor connections with TEVENT_FD_ERROR
       via  c5201cd0b59 s3:rpc_server: make use of tstream_bsd_fail_readv_first_error(true)
       via  7e6f830d9d3 s4:rpc_server: make use of tstream_bsd_fail_readv_first_error(true)
       via  27b2ca7d8d7 s4:service_named_pipe: make use of tstream_bsd_fail_readv_first_error(true)
       via  8e8f2fa9c7f libcli/named_pipe_auth: let tstream_npa_existing_socket use tstream_bsd_fail_readv_first_error(true)
       via  82b2a379e8f s4:wrepl_server: make use of tstream_bsd_fail_readv_first_error(true)
       via  0e83b564039 s4:libcli/wrepl: make use of tstream_bsd_fail_readv_first_error(false)
       via  391ef8ae7b2 s4:ntp_signd: make use of tstream_bsd_fail_readv_first_error(true)
       via  d9c416baa99 s3:libsmb: the unexpected handler use tstream_bsd_fail_readv_first_error(true)
       via  341e800dfe8 s4:dns_server: make use of tstream_bsd_fail_readv_first_error(true)
       via  e897ccd9c8a s4:ldap_server: make use of tstream_bsd_fail_readv_first_error(true)
       via  3a47a276fde s4:kdc: make use of tstream_bsd_fail_readv_first_error(true)
       via  71e8727bdc0 lib/tsocket: add tstream_bsd_fail_readv_first_error()
       via  5bedf1675e7 lib/tsocket: make use of TEVENT_FD_ERROR in tstream_bsd_fde_handler()
       via  22e3a542f39 lib/tsocket: let tstream_bsd_connect_send() use TEVENT_FD_ERROR instead of TEVENT_FD_READ
       via  82aafa4ac8b lib/async_req: let writev_send/recv use TEVENT_FD_ERROR
       via  21a18a5b52a lib/async_req: let async_connect_send use TEVENT_FD_ERROR instead of TEVENT_FD_READ
       via  66b25637220 lib/tsocket: make use of samba_socket_sock_error()
       via  cd964e521ba lib/tsocket: make use of samba_socket_poll_or_sock_error()
       via  f8213ec8710 lib/util: add samba_socket_{poll,sock,poll_or_sock}_error()
      from  63aeb64504c s4:kdc: Add device to Authenticated Users for authentication policy evaluation

https://git.samba.org/?p=samba.git;a=shortlog;h=master


- Log -----------------------------------------------------------------
commit 3f4f80edba2156492645900527d628b1fab5ca4a
Author: Stefan Metzmacher <metze at samba.org>
Date:   Thu Jan 12 11:35:30 2023 +0100

    smb2_server: monitor connections with TEVENT_FD_ERROR
    
    By asking for TEVENT_FD_ERROR we're able to fail early
    when a connection to a client is broken.
    
    In that case it does not make any sense to process
    pending requests in the recv queue as it's not
    possible to deliver the response to the client anyway.
    
    Signed-off-by: Stefan Metzmacher <metze at samba.org>
    Reviewed-by: Ralph Boehme <slow at samba.org>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>
    
    Autobuild-User(master): Ralph Böhme <slow at samba.org>
    Autobuild-Date(master): Tue Oct 24 10:32:56 UTC 2023 on atb-devel-224

commit c5201cd0b59647c41ac46ed1f4efb1a72bc37bf9
Author: Stefan Metzmacher <metze at samba.org>
Date:   Thu Jan 12 10:49:13 2023 +0100

    s3:rpc_server: make use of tstream_bsd_fail_readv_first_error(true)
    
    This avoids doing useless work in case the client connection
    is already broken.
    
    Signed-off-by: Stefan Metzmacher <metze at samba.org>
    Reviewed-by: Ralph Boehme <slow at samba.org>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>

commit 7e6f830d9d3aac14336a886c1c5d9ff623218085
Author: Stefan Metzmacher <metze at samba.org>
Date:   Thu Jan 12 10:48:22 2023 +0100

    s4:rpc_server: make use of tstream_bsd_fail_readv_first_error(true)
    
    This avoids doing useless work in case the client connection
    is already broken.
    
    Signed-off-by: Stefan Metzmacher <metze at samba.org>
    Reviewed-by: Ralph Boehme <slow at samba.org>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>

commit 27b2ca7d8d725b374aef97c11e0650686cfadbd3
Author: Stefan Metzmacher <metze at samba.org>
Date:   Thu Jan 12 10:46:56 2023 +0100

    s4:service_named_pipe: make use of tstream_bsd_fail_readv_first_error(true)
    
    This avoids doing useless work in case the client connection
    is already broken.
    
    Signed-off-by: Stefan Metzmacher <metze at samba.org>
    Reviewed-by: Ralph Boehme <slow at samba.org>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>

commit 8e8f2fa9c7f1e2cb8a296755a8c0aba6a2d22b54
Author: Stefan Metzmacher <metze at samba.org>
Date:   Thu Jan 12 10:44:25 2023 +0100

    libcli/named_pipe_auth: let tstream_npa_existing_socket use tstream_bsd_fail_readv_first_error(true)
    
    This avoids doing useless work in case the client connection
    is already broken.
    
    Signed-off-by: Stefan Metzmacher <metze at samba.org>
    Reviewed-by: Ralph Boehme <slow at samba.org>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>

commit 82b2a379e8fac89d94a1321f4df7d732f4fbfc5d
Author: Stefan Metzmacher <metze at samba.org>
Date:   Thu Jan 12 10:46:20 2023 +0100

    s4:wrepl_server: make use of tstream_bsd_fail_readv_first_error(true)
    
    This avoids doing useless work in case the client connection
    is already broken.
    
    Signed-off-by: Stefan Metzmacher <metze at samba.org>
    Reviewed-by: Ralph Boehme <slow at samba.org>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>

commit 0e83b5640398a92fb0bf063f902c53801eaec92f
Author: Stefan Metzmacher <metze at samba.org>
Date:   Thu Jan 12 10:43:21 2023 +0100

    s4:libcli/wrepl: make use of tstream_bsd_fail_readv_first_error(false)
    
    As a client we want recv pending responses even if the server
    already closed the connection.
    
    While tstream_bsd_fail_readv_first_error(false) is the default for
    tstream_bsd, the wins replication protocol is special as it has
    a way to switch server and client roles on an existing tcp connection.
    
    Signed-off-by: Stefan Metzmacher <metze at samba.org>
    Reviewed-by: Ralph Boehme <slow at samba.org>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>

commit 391ef8ae7b2d394e65b8872dcca67984673e7574
Author: Stefan Metzmacher <metze at samba.org>
Date:   Thu Jan 12 10:42:14 2023 +0100

    s4:ntp_signd: make use of tstream_bsd_fail_readv_first_error(true)
    
    This avoids doing useless work in case the client connection
    is already broken.
    
    Signed-off-by: Stefan Metzmacher <metze at samba.org>
    Reviewed-by: Ralph Boehme <slow at samba.org>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>

commit d9c416baa998572236510a554769eb9377d3ceeb
Author: Stefan Metzmacher <metze at samba.org>
Date:   Thu Jan 12 10:41:04 2023 +0100

    s3:libsmb: the unexpected handler use tstream_bsd_fail_readv_first_error(true)
    
    This avoids doing useless work in case the client connection
    is already broken.
    
    Signed-off-by: Stefan Metzmacher <metze at samba.org>
    Reviewed-by: Ralph Boehme <slow at samba.org>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>

commit 341e800dfe82bbe886e6370f952836590ad202e0
Author: Stefan Metzmacher <metze at samba.org>
Date:   Thu Jan 12 10:40:13 2023 +0100

    s4:dns_server: make use of tstream_bsd_fail_readv_first_error(true)
    
    This avoids doing useless work in case the client connection
    is already broken.
    
    Signed-off-by: Stefan Metzmacher <metze at samba.org>
    Reviewed-by: Ralph Boehme <slow at samba.org>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>

commit e897ccd9c8adb64a412d4ea67a58efe493b91910
Author: Stefan Metzmacher <metze at samba.org>
Date:   Thu Jan 12 10:39:48 2023 +0100

    s4:ldap_server: make use of tstream_bsd_fail_readv_first_error(true)
    
    This avoids doing useless work in case the client connection
    is already broken.
    
    Signed-off-by: Stefan Metzmacher <metze at samba.org>
    Reviewed-by: Ralph Boehme <slow at samba.org>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>

commit 3a47a276fdec7ab2610023d220fb05c6a15d790f
Author: Stefan Metzmacher <metze at samba.org>
Date:   Thu Jan 12 10:38:53 2023 +0100

    s4:kdc: make use of tstream_bsd_fail_readv_first_error(true)
    
    This avoids doing useless work in case the client connection
    is already broken.
    
    Signed-off-by: Stefan Metzmacher <metze at samba.org>
    Reviewed-by: Ralph Boehme <slow at samba.org>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>

commit 71e8727bdc085673360f69f6e28afbd6a6adfae7
Author: Stefan Metzmacher <metze at samba.org>
Date:   Thu Jan 12 10:08:56 2023 +0100

    lib/tsocket: add tstream_bsd_fail_readv_first_error()
    
    This gives the caller the option to fail immediately if
    TEVENT_FD_ERROR appear even with pending bytes in the
    recv queue.
    
    Servers typically want to activate this in order to avoid
    pointless work, while clients typically want to read
    pending responses from the recv queue.
    
    Signed-off-by: Stefan Metzmacher <metze at samba.org>
    Reviewed-by: Ralph Boehme <slow at samba.org>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>

commit 5bedf1675e7b5a960f34beef87da79cad5b696a6
Author: Stefan Metzmacher <metze at samba.org>
Date:   Wed Jan 11 20:17:06 2023 +0100

    lib/tsocket: make use of TEVENT_FD_ERROR in tstream_bsd_fde_handler()
    
    This makes the logic introduced to fix bug #15202 simpler.
    
    While developing this I noticed that a lot of callers
    rely on the fact that they can read the pending bytes out
    of the recv queue before EOF is reported.
    
    So I changed the code handle TEVENT_FD_ERROR together with
    TEVENT_FD_READ in a way that keep the existing callers happy.
    
    In the next step we'll add a way to let callers opt-in in order
    to fail immediately if TEVENT_FD_ERROR appears (even if there
    are pending bytes remaining in the recv queue).
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=15202
    
    Signed-off-by: Stefan Metzmacher <metze at samba.org>
    Reviewed-by: Ralph Boehme <slow at samba.org>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>

commit 22e3a542f39ce3f88c22cf871ebc3228acfc19ba
Author: Stefan Metzmacher <metze at samba.org>
Date:   Wed Jan 11 20:15:33 2023 +0100

    lib/tsocket: let tstream_bsd_connect_send() use TEVENT_FD_ERROR instead of TEVENT_FD_READ
    
    This mostly cosmetic, but now that we have TEVENT_FD_ERROR we should use it.
    
    Signed-off-by: Stefan Metzmacher <metze at samba.org>
    Reviewed-by: Ralph Boehme <slow at samba.org>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>

commit 82aafa4ac8b9751a044eaaa1796b15b0a88d1e6a
Author: Stefan Metzmacher <metze at samba.org>
Date:   Thu Jan 12 12:54:33 2023 +0100

    lib/async_req: let writev_send/recv use TEVENT_FD_ERROR
    
    Unless err_on_readability is true, we use TEVENT_FD_READ only
    to detect errors. Now that we have TEVENT_FD_ERROR we should use it.
    
    As a side effect it makes the code much simpler and clearer, as
    we can directly map TEVENT_FD_ERROR to EPIPE.
    
    In addition the err_on_readability=true case is now also
    clearer, where we just map TEVENT_FD_READ to EPIPE.
    
    Signed-off-by: Stefan Metzmacher <metze at samba.org>
    Reviewed-by: Ralph Boehme <slow at samba.org>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>

commit 21a18a5b52ad5dba3ff54c829d852c2a65244d7e
Author: Stefan Metzmacher <metze at samba.org>
Date:   Wed Jan 11 20:04:26 2023 +0100

    lib/async_req: let async_connect_send use TEVENT_FD_ERROR instead of TEVENT_FD_READ
    
    This mostly cosmetic, but now that we have TEVENT_FD_ERROR we should use it.
    
    Signed-off-by: Stefan Metzmacher <metze at samba.org>
    Reviewed-by: Ralph Boehme <slow at samba.org>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>

commit 66b2563722013f4a15c34a1117c98f562bb2a65c
Author: Stefan Metzmacher <metze at samba.org>
Date:   Thu Jan 12 11:35:11 2023 +0100

    lib/tsocket: make use of samba_socket_sock_error()
    
    This is nicer than calling getsockopt(state->fd, SOL_SOCKET, SO_ERROR)
    directly.
    
    Signed-off-by: Stefan Metzmacher <metze at samba.org>
    Reviewed-by: Ralph Boehme <slow at samba.org>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>

commit cd964e521ba7bd76fc3d39d2025d9b3c02082050
Author: Stefan Metzmacher <metze at samba.org>
Date:   Thu Jan 12 11:35:11 2023 +0100

    lib/tsocket: make use of samba_socket_poll_or_sock_error()
    
    This is just a copy of the existing code...
    
    Signed-off-by: Stefan Metzmacher <metze at samba.org>
    Reviewed-by: Ralph Boehme <slow at samba.org>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>

commit f8213ec871016cfa380b85334192cc72589ddb7f
Author: Stefan Metzmacher <metze at samba.org>
Date:   Thu Jan 12 11:14:06 2023 +0100

    lib/util: add samba_socket_{poll,sock,poll_or_sock}_error()
    
    These are copies of the static functions in lib/tsocket/tsocket_bsd.c,
    which we will replace in the next commit.
    
    Signed-off-by: Stefan Metzmacher <metze at samba.org>
    Reviewed-by: Ralph Boehme <slow at samba.org>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>

-----------------------------------------------------------------------

Summary of changes:
 lib/async_req/async_sock.c                 |  52 ++---
 lib/tsocket/tsocket.h                      |  22 ++
 lib/tsocket/tsocket_bsd.c                  | 347 +++++++++++------------------
 lib/util/util_net.c                        |  99 ++++++++
 lib/util/util_net.h                        |  14 ++
 libcli/named_pipe_auth/npa_tstream.c       |   2 +
 source3/libsmb/unexpected.c                |   2 +
 source3/rpc_server/rpc_host.c              |   2 +
 source3/rpc_server/rpc_worker.c            |   2 +
 source3/smbd/smb2_server.c                 |  19 +-
 source4/dns_server/dns_server.c            |   2 +
 source4/kdc/kdc-server.c                   |   2 +
 source4/ldap_server/ldap_server.c          |   2 +
 source4/libcli/wrepl/winsrepl.c            |   2 +
 source4/ntp_signd/ntp_signd.c              |   2 +
 source4/rpc_server/dcerpc_server.c         |   2 +
 source4/samba/service_named_pipe.c         |   2 +
 source4/wrepl_server/wrepl_in_connection.c |   5 +
 18 files changed, 333 insertions(+), 247 deletions(-)


Changeset truncated at 500 lines:

diff --git a/lib/async_req/async_sock.c b/lib/async_req/async_sock.c
index ae1d3257f65..bdbefd162ac 100644
--- a/lib/async_req/async_sock.c
+++ b/lib/async_req/async_sock.c
@@ -151,7 +151,8 @@ struct tevent_req *async_connect_send(
 	 * use TEVENT_FD_READ in addition until we have
 	 * TEVENT_FD_ERROR.
 	 */
-	state->fde = tevent_add_fd(ev, state, fd, TEVENT_FD_READ|TEVENT_FD_WRITE,
+	state->fde = tevent_add_fd(ev, state, fd,
+				   TEVENT_FD_ERROR|TEVENT_FD_WRITE,
 				   async_connect_connected, req);
 	if (state->fde == NULL) {
 		tevent_req_error(req, ENOMEM);
@@ -277,8 +278,10 @@ struct tevent_req *writev_send(TALLOC_CTX *mem_ctx, struct tevent_context *ev,
 	if (tevent_req_nomem(state->iov, req)) {
 		return tevent_req_post(req, ev);
 	}
-	state->flags = TEVENT_FD_WRITE|TEVENT_FD_READ;
-	state->err_on_readability = err_on_readability;
+	state->flags = TEVENT_FD_WRITE | TEVENT_FD_ERROR;
+	if (err_on_readability) {
+		state->flags |= TEVENT_FD_READ;
+	}
 
 	tevent_req_set_cleanup_fn(req, writev_cleanup);
 	tevent_req_set_cancel_fn(req, writev_cancel);
@@ -401,36 +404,21 @@ static void writev_handler(struct tevent_context *ev, struct tevent_fd *fde,
 	struct writev_state *state =
 		tevent_req_data(req, struct writev_state);
 
-	if ((state->flags & TEVENT_FD_READ) && (flags & TEVENT_FD_READ)) {
-		int ret, value;
-
-		if (state->err_on_readability) {
-			/* Readable and the caller wants an error on read. */
-			tevent_req_error(req, EPIPE);
-			return;
-		}
+	if (flags & TEVENT_FD_ERROR) {
+		/*
+		 * There's an error, for legacy reasons
+		 * we just use EPIPE instead of a more
+		 * detailed error using
+		 * samba_socket_poll_or_sock_error().
+		 */
+		tevent_req_error(req, EPIPE);
+		return;
+	}
 
-		/* Might be an error. Check if there are bytes to read */
-		ret = ioctl(state->fd, FIONREAD, &value);
-		/* FIXME - should we also check
-		   for ret == 0 and value == 0 here ? */
-		if (ret == -1) {
-			/* There's an error. */
-			tevent_req_error(req, EPIPE);
-			return;
-		}
-		/* A request for TEVENT_FD_READ will succeed from now and
-		   forevermore until the bytes are read so if there was
-		   an error we'll wait until we do read, then get it in
-		   the read callback function. Until then, remove TEVENT_FD_READ
-		   from the flags we're waiting for. */
-		state->flags &= ~TEVENT_FD_READ;
-		TEVENT_FD_NOT_READABLE(fde);
-
-		/* If not writable, we're done. */
-		if (!(flags & TEVENT_FD_WRITE)) {
-			return;
-		}
+	if (flags & TEVENT_FD_READ) {
+		/* Readable and the caller wants an error on read. */
+		tevent_req_error(req, EPIPE);
+		return;
 	}
 
 	writev_do(req, state);
diff --git a/lib/tsocket/tsocket.h b/lib/tsocket/tsocket.h
index 22eb758bccd..07e10c84a8a 100644
--- a/lib/tsocket/tsocket.h
+++ b/lib/tsocket/tsocket.h
@@ -844,6 +844,28 @@ int _tdgram_unix_socket(const struct tsocket_address *local,
 bool tstream_bsd_optimize_readv(struct tstream_context *stream,
 				bool on);
 
+/**
+ * @brief Request that tstream_readv_send() fails within pending data
+ *
+ * By default we allow pending data to be drained from the
+ * recv queue, before we report EPIPE when reaching EOF.
+ *
+ * For server applications it's typically useful to
+ * fail early in order to avoid useless work,
+ * as the response can't be transferred to the client anyway.
+ *
+ * @param[in]  stream   The tstream_context of a bsd socket, if this
+ *                      not a bsd socket the function does nothing.
+ *
+ * @param[in]  on       The boolean value to turn the early fail on and off.
+ *
+ * @return              The old boolean value.
+ *
+ * @see tstream_readv_send()
+ */
+bool tstream_bsd_fail_readv_first_error(struct tstream_context *stream,
+					bool on);
+
 /**
  * @brief Connect async to a TCP endpoint and create a tstream_context for the
  * stream based communication.
diff --git a/lib/tsocket/tsocket_bsd.c b/lib/tsocket/tsocket_bsd.c
index daba33eb9d7..4483b03b19f 100644
--- a/lib/tsocket/tsocket_bsd.c
+++ b/lib/tsocket/tsocket_bsd.c
@@ -24,10 +24,8 @@
 #include "replace.h"
 #include "system/filesys.h"
 #include "system/network.h"
-#include "system/select.h"
 #include "tsocket.h"
 #include "tsocket_internal.h"
-#include "lib/util/select.h"
 #include "lib/util/iov_buf.h"
 #include "lib/util/blocking.h"
 #include "lib/util/util_net.h"
@@ -173,103 +171,6 @@ static ssize_t tsocket_bsd_netlink_pending(int fd)
 }
 #endif
 
-static int tsocket_bsd_poll_error(int fd)
-{
-	struct pollfd pfd = {
-		.fd = fd,
-#ifdef POLLRDHUP
-		.events = POLLRDHUP, /* POLLERR and POLLHUP are not needed */
-#endif
-	};
-	int ret;
-
-	errno = 0;
-	ret = sys_poll_intr(&pfd, 1, 0);
-	if (ret == 0) {
-		return 0;
-	}
-	if (ret != 1) {
-		return POLLNVAL;
-	}
-
-	if (pfd.revents & POLLERR) {
-		return POLLERR;
-	}
-	if (pfd.revents & POLLHUP) {
-		return POLLHUP;
-	}
-#ifdef POLLRDHUP
-	if (pfd.revents & POLLRDHUP) {
-		return POLLRDHUP;
-	}
-#endif
-
-	/* should never be reached! */
-	return POLLNVAL;
-}
-
-static int tsocket_bsd_sock_error(int fd)
-{
-	int ret, error = 0;
-	socklen_t len = sizeof(error);
-
-	/*
-	 * if no data is available check if the socket is in error state. For
-	 * dgram sockets it's the way to return ICMP error messages of
-	 * connected sockets to the caller.
-	 */
-	ret = getsockopt(fd, SOL_SOCKET, SO_ERROR, &error, &len);
-	if (ret == -1) {
-		return ret;
-	}
-	if (error != 0) {
-		errno = error;
-		return -1;
-	}
-	return 0;
-}
-
-static int tsocket_bsd_error(int fd)
-{
-	int ret;
-	int poll_error = 0;
-
-	poll_error = tsocket_bsd_poll_error(fd);
-	if (poll_error == 0) {
-		return 0;
-	}
-
-#ifdef POLLRDHUP
-	if (poll_error == POLLRDHUP) {
-		errno = ECONNRESET;
-		return -1;
-	}
-#endif
-
-	if (poll_error == POLLHUP) {
-		errno = EPIPE;
-		return -1;
-	}
-
-	/*
-	 * POLLERR and POLLNVAL fallback to
-	 * getsockopt(fd, SOL_SOCKET, SO_ERROR)
-	 * and force EPIPE as fallback.
-	 */
-
-	errno = 0;
-	ret = tsocket_bsd_sock_error(fd);
-	if (ret == 0) {
-		errno = EPIPE;
-	}
-
-	if (errno == 0) {
-		errno = EPIPE;
-	}
-
-	return -1;
-}
-
 static ssize_t tsocket_bsd_pending(int fd)
 {
 	int ret;
@@ -290,7 +191,7 @@ static ssize_t tsocket_bsd_pending(int fd)
 		return value;
 	}
 
-	return tsocket_bsd_error(fd);
+	return samba_socket_poll_or_sock_error(fd);
 }
 
 static const struct tsocket_address_ops tsocket_address_bsd_ops;
@@ -1749,14 +1650,12 @@ struct tstream_bsd {
 	void *event_ptr;
 	struct tevent_fd *fde;
 	bool optimize_readv;
+	bool fail_readv_first_error;
 
 	void *readable_private;
 	void (*readable_handler)(void *private_data);
 	void *writeable_private;
 	void (*writeable_handler)(void *private_data);
-
-	struct tevent_context *error_ctx;
-	struct tevent_timer *error_timer;
 };
 
 bool tstream_bsd_optimize_readv(struct tstream_context *stream,
@@ -1778,26 +1677,23 @@ bool tstream_bsd_optimize_readv(struct tstream_context *stream,
 	return old;
 }
 
-static void tstream_bsd_error_timer(struct tevent_context *ev,
-				    struct tevent_timer *te,
-				    struct timeval current_time,
-				    void *private_data)
+bool tstream_bsd_fail_readv_first_error(struct tstream_context *stream,
+					bool on)
 {
 	struct tstream_bsd *bsds =
-		talloc_get_type(private_data,
+		talloc_get_type(_tstream_context_data(stream),
 		struct tstream_bsd);
+	bool old;
 
-	TALLOC_FREE(bsds->error_timer);
-
-	/*
-	 * Turn on TEVENT_FD_READABLE() again
-	 * if we have a writeable_handler that
-	 * wants to monitor the connection
-	 * for errors.
-	 */
-	if (bsds->writeable_handler != NULL) {
-		TEVENT_FD_READABLE(bsds->fde);
+	if (bsds == NULL) {
+		/* not a bsd socket */
+		return false;
 	}
+
+	old = bsds->fail_readv_first_error;
+	bsds->fail_readv_first_error = on;
+
+	return old;
 }
 
 static void tstream_bsd_fde_handler(struct tevent_context *ev,
@@ -1808,80 +1704,106 @@ static void tstream_bsd_fde_handler(struct tevent_context *ev,
 	struct tstream_bsd *bsds = talloc_get_type_abort(private_data,
 				   struct tstream_bsd);
 
+	if (flags & TEVENT_FD_ERROR) {
+		/*
+		 * We lazily keep TEVENT_FD_READ alive
+		 * in tstream_bsd_set_readable_handler()
+		 *
+		 * So we have to check TEVENT_FD_READ
+		 * as well as bsds->readable_handler
+		 *
+		 * We only drain remaining data from the
+		 * the recv queue if available and desired.
+		 */
+		if ((flags & TEVENT_FD_READ) &&
+		    !bsds->fail_readv_first_error &&
+		    (bsds->readable_handler != NULL))
+		{
+			/*
+			 * If there's still data to read
+			 * we allow it to be read until
+			 * we reach EOF (=> EPIPE).
+			 */
+			bsds->readable_handler(bsds->readable_private);
+			return;
+		}
+
+		/*
+		 * If there's no data left to read,
+		 * we get the error.
+		 *
+		 * It means we no longer call any readv or
+		 * writev, as bsds->error is checked first.
+		 */
+		if (bsds->error == 0) {
+			int ret = samba_socket_poll_or_sock_error(bsds->fd);
+
+			if (ret == -1) {
+				bsds->error = errno;
+			}
+			/* fallback to EPIPE */
+			if (bsds->error == 0) {
+				bsds->error = EPIPE;
+			}
+		}
+
+		/*
+		 * Let write to fail early.
+		 *
+		 * Note we only need to check TEVENT_FD_WRITE
+		 * as tstream_bsd_set_writeable_handler()
+		 * clear it together with the handler.
+		 */
+		if (flags & TEVENT_FD_WRITE) {
+			bsds->writeable_handler(bsds->writeable_private);
+			return;
+		}
+
+		/* We prefer the readable handler to fire first. */
+		if (bsds->readable_handler != NULL) {
+			bsds->readable_handler(bsds->readable_private);
+			return;
+		}
+
+		/* As last resort we notify the writeable handler */
+		if (bsds->writeable_handler != NULL) {
+			bsds->writeable_handler(bsds->writeable_private);
+			return;
+		}
+
+		/*
+		 * We may hit this because we don't clear TEVENT_FD_ERROR
+		 * in tstream_bsd_set_readable_handler() nor
+		 * tstream_bsd_set_writeable_handler().
+		 *
+		 * As we already captured the error, we can remove
+		 * the fde completely.
+		 */
+		TALLOC_FREE(bsds->fde);
+		return;
+	}
 	if (flags & TEVENT_FD_WRITE) {
 		bsds->writeable_handler(bsds->writeable_private);
 		return;
 	}
 	if (flags & TEVENT_FD_READ) {
 		if (!bsds->readable_handler) {
-			struct timeval recheck_time;
-
 			/*
+			 * tstream_bsd_set_readable_handler
+			 * doesn't clear TEVENT_FD_READ.
+			 *
 			 * In order to avoid cpu-spinning
-			 * we no longer want to get TEVENT_FD_READ
+			 * we need to clear it here.
 			 */
 			TEVENT_FD_NOT_READABLE(bsds->fde);
 
-			if (!bsds->writeable_handler) {
-				return;
-			}
-
 			/*
-			 * If we have a writeable handler we
-			 * want that to report connection errors
-			 * early.
-			 *
-			 * So we check if the socket is in an
-			 * error state.
+			 * Here we're lazy and keep TEVENT_FD_ERROR
+			 * alive. If it's triggered the next time
+			 * we'll handle it gracefully above
+			 * and end up with TALLOC_FREE(bsds->fde);
+			 * in order to spin on TEVENT_FD_ERROR.
 			 */
-			if (bsds->error == 0) {
-				int ret = tsocket_bsd_error(bsds->fd);
-
-				if (ret == -1) {
-					bsds->error = errno;
-				}
-			}
-
-			if (bsds->error != 0) {
-				/*
-				 * Let the writeable handler report the error
-				 */
-				bsds->writeable_handler(bsds->writeable_private);
-				return;
-			}
-
-			/*
-			 * Here we called TEVENT_FD_NOT_READABLE() without
-			 * calling into the writeable handler.
-			 *
-			 * So we may have to wait for the kernels tcp stack
-			 * to report TEVENT_FD_WRITE in order to let
-			 * make progress and turn on TEVENT_FD_READABLE()
-			 * again.
-			 *
-			 * As a fallback we use a timer that turns on
-			 * TEVENT_FD_READABLE() again after a timeout of
-			 * 1 second.
-			 */
-
-			if (bsds->error_timer != NULL) {
-				return;
-			}
-
-			recheck_time = timeval_current_ofs(1, 0);
-			bsds->error_timer = tevent_add_timer(bsds->error_ctx,
-							     bsds,
-							     recheck_time,
-							     tstream_bsd_error_timer,
-							     bsds);
-			if (bsds->error_timer == NULL) {
-				bsds->error = ENOMEM;
-				/*
-				 * Let the writeable handler report the error
-				 */
-				bsds->writeable_handler(bsds->writeable_private);
-				return;
-			}
 			return;
 		}
 		bsds->readable_handler(bsds->readable_private);
@@ -1905,6 +1827,11 @@ static int tstream_bsd_set_readable_handler(struct tstream_bsd *bsds,
 		bsds->readable_handler = NULL;
 		bsds->readable_private = NULL;
 
+		/*
+		 * Here we are lazy as it's very likely that the next
+		 * tevent_readv_send() will come in shortly,
+		 * so we keep TEVENT_FD_READ alive.
+		 */
 		return 0;
 	}
 
@@ -1922,7 +1849,8 @@ static int tstream_bsd_set_readable_handler(struct tstream_bsd *bsds,
 		TALLOC_FREE(bsds->fde);
 
 		bsds->fde = tevent_add_fd(ev, bsds,
-					  bsds->fd, TEVENT_FD_READ,
+					  bsds->fd,
+					  TEVENT_FD_ERROR | TEVENT_FD_READ,
 					  tstream_bsd_fde_handler,
 					  bsds);
 		if (!bsds->fde) {
@@ -1934,10 +1862,13 @@ static int tstream_bsd_set_readable_handler(struct tstream_bsd *bsds,
 		bsds->event_ptr = ev;
 	} else if (!bsds->readable_handler) {
 		TEVENT_FD_READABLE(bsds->fde);
+		/*
+		 * TEVENT_FD_ERROR is likely already set, so
+		 * TEVENT_FD_WANTERROR() is most likely a no-op.
+		 */
+		TEVENT_FD_WANTERROR(bsds->fde);
 	}
 
-	TALLOC_FREE(bsds->error_timer);
-
 	bsds->readable_handler = handler;
 	bsds->readable_private = private_data;
 


-- 
Samba Shared Repository



More information about the samba-cvs mailing list