From: Nick Mathewson <nickm@torproject.org>
Date: Thu, 29 Sep 2011 13:30:04 +0000 (-0400)
Subject: Prefer mmap to sendfile unless a DRAINS_TO_FD flag is set. Allows add_file to work... 
X-Git-Tag: release-2.0.15-stable~4^2
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=0ba0af9c6c3dbf4dfec8c3cd53b0d90b49153016;p=libevent

Prefer mmap to sendfile unless a DRAINS_TO_FD flag is set. Allows add_file to work with SSL.

The sendfile() implementation for evbuffer_add_file is potentially more
efficient, but it has a problem: you can only use it to send bytes over
a socket using sendfile().  If you are writing bytes via SSL_send() or
via a filter, or if you need to be able to inspect your buffer, it
doesn't work.

As an easy fix, this patch disables the sendfile-based implementation of
evbuffer_add_file on an evbuffer unless the user sets a new
EVBUFFER_FLAG_DRAINS_TO_FD flag on that evbuffer, indicating that the
evbuffer will not be inspected, but only written out via
evbuffer_write(), evbuffer_write_atmost(), or drained with stuff like
evbuffer_drain() or evbuffer_add_buffer().  This flag is off by
default, except for evbuffers used for output on bufferevent_socket.

In the future, it could be interesting to make a best-effort file
segment implementation that tries to send via sendfile, but mmaps on
demand.  That's too much complexity for a stable release series, though.
---

diff --git a/buffer.c b/buffer.c
index 146bbdf4..e404f2b4 100644
--- a/buffer.c
+++ b/buffer.c
@@ -327,6 +327,24 @@ evbuffer_new(void)
 	return (buffer);
 }
 
+int
+evbuffer_set_flags(struct evbuffer *buf, ev_uint64_t flags)
+{
+	EVBUFFER_LOCK(buf);
+	buf->flags |= (ev_uint32_t)flags;
+	EVBUFFER_UNLOCK(buf);
+	return 0;
+}
+
+int
+evbuffer_clear_flags(struct evbuffer *buf, ev_uint64_t flags)
+{
+	EVBUFFER_LOCK(buf);
+	buf->flags &= ~(ev_uint32_t)flags;
+	EVBUFFER_UNLOCK(buf);
+	return 0;
+}
+
 void
 _evbuffer_incref(struct evbuffer *buf)
 {
@@ -2680,11 +2698,20 @@ evbuffer_add_file(struct evbuffer *outbuf, int fd,
 #if defined(USE_SENDFILE) || defined(_EVENT_HAVE_MMAP)
 	struct evbuffer_chain *chain;
 	struct evbuffer_chain_fd *info;
+#endif
+#if defined(USE_SENDFILE)
+	int sendfile_okay = 1;
 #endif
 	int ok = 1;
 
 #if defined(USE_SENDFILE)
 	if (use_sendfile) {
+		EVBUFFER_LOCK(outbuf);
+		sendfile_okay = outbuf->flags & EVBUFFER_FLAG_DRAINS_TO_FD;
+		EVBUFFER_UNLOCK(outbuf);
+	}
+
+	if (use_sendfile && sendfile_okay) {
 		chain = evbuffer_chain_new(sizeof(struct evbuffer_chain_fd));
 		if (chain == NULL) {
 			event_warn("%s: out of memory", __func__);
diff --git a/bufferevent_sock.c b/bufferevent_sock.c
index 089aedb4..1dc0f330 100644
--- a/bufferevent_sock.c
+++ b/bufferevent_sock.c
@@ -328,6 +328,7 @@ bufferevent_socket_new(struct event_base *base, evutil_socket_t fd,
 		return NULL;
 	}
 	bufev = &bufev_p->bev;
+	evbuffer_set_flags(bufev->output, EVBUFFER_FLAG_DRAINS_TO_FD);
 
 	event_assign(&bufev->ev_read, bufev->ev_base, fd,
 	    EV_READ|EV_PERSIST, bufferevent_readcb, bufev);
diff --git a/evbuffer-internal.h b/evbuffer-internal.h
index 5879f874..89139602 100644
--- a/evbuffer-internal.h
+++ b/evbuffer-internal.h
@@ -129,6 +129,8 @@ struct evbuffer {
 	/** True iff this buffer is set up for overlapped IO. */
 	unsigned is_overlapped : 1;
 #endif
+	/** Zero or more EVBUFFER_FLAG_* bits */
+	ev_uint32_t flags;
 
 	/** Used to implement deferred callbacks. */
 	struct deferred_cb_queue *cb_queue;
diff --git a/include/event2/buffer.h b/include/event2/buffer.h
index 19f6f90a..c53cb9c9 100644
--- a/include/event2/buffer.h
+++ b/include/event2/buffer.h
@@ -141,7 +141,6 @@ struct evbuffer_iovec {
 	occurred
  */
 struct evbuffer *evbuffer_new(void);
-
 /**
   Deallocate storage for an evbuffer.
 
@@ -175,6 +174,41 @@ void evbuffer_lock(struct evbuffer *buf);
 */
 void evbuffer_unlock(struct evbuffer *buf);
 
+
+/** If this flag is set, then we will not use evbuffer_peek(),
+ * evbuffer_remove(), evbuffer_remove_buffer(), and so on to read bytes
+ * from this buffer: we'll only take bytes out of this buffer by
+ * writing them to the network (as with evbuffer_write_atmost), by
+ * removing them without observing them (as with evbuffer_drain),
+ * or by copying them all out at once (as with evbuffer_add_buffer).
+ *
+ * Using this option allows the implementation to use sendfile-based
+ * operations for evbuffer_add_file(); see that function for more
+ * information.
+ *
+ * This flag is on by default for bufferevents that can take advantage
+ * of it; you should never actually need to set it on a bufferevent's
+ * output buffer.
+ */
+#define EVBUFFER_FLAG_DRAINS_TO_FD 1
+
+/** Change the flags that are set for an evbuffer by adding more.
+ *
+ * @param buffer the evbuffer that the callback is watching.
+ * @param cb the callback whose status we want to change.
+ * @param flags One or more EVBUFFER_FLAG_* options
+ * @return 0 on success, -1 on failure.
+ */
+int evbuffer_set_flags(struct evbuffer *buf, ev_uint64_t flags);
+/** Change the flags that are set for an evbuffer by removing some.
+ *
+ * @param buffer the evbuffer that the callback is watching.
+ * @param cb the callback whose status we want to change.
+ * @param flags One or more EVBUFFER_FLAG_* options
+ * @return 0 on success, -1 on failure.
+ */
+int evbuffer_clear_flags(struct evbuffer *buf, ev_uint64_t flags);
+
 /**
   Returns the total number of bytes stored in the evbuffer
 
@@ -408,8 +442,9 @@ int evbuffer_add_reference(struct evbuffer *outbuf,
   Copy data from a file into the evbuffer for writing to a socket.
 
   This function avoids unnecessary data copies between userland and
-  kernel.  Where available, it uses sendfile or splice; failing those,
-  it tries to use mmap.
+  kernel.  If sendfile is available and the EVBUFFER_FLAG_DRAINS_TO_FD
+  flag is set, it uses those functions.  Otherwise, it tries to use
+  mmap (or CreateFileMapping on Windows).
 
   The function owns the resulting file descriptor and will close it
   when finished transferring data.
diff --git a/test/regress_buffer.c b/test/regress_buffer.c
index 4deaef57..e6b8dfcb 100644
--- a/test/regress_buffer.c
+++ b/test/regress_buffer.c
@@ -618,6 +618,9 @@ test_evbuffer_add_file(void *ptr)
 		TT_DIE(("Didn't recognize the implementation"));
 	}
 
+	/* Say that it drains to a fd so that we can use sendfile. */
+	evbuffer_set_flags(src, EVBUFFER_FLAG_DRAINS_TO_FD);
+
 #if defined(_EVENT_HAVE_SENDFILE) && defined(__sun__) && defined(__svr4__)
 	/* We need to use a pair of AF_INET sockets, since Solaris
 	   doesn't support sendfile() over AF_UNIX. */