You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apr.apache.org by dean gaudet <de...@arctic.org> on 2001/07/07 20:23:42 UTC

Re: Extraneous socket read?

On Mon, 2 Jul 2001, Brian Pane wrote:

> >On Mon, 2 Jul 2001, Brian Pane wrote:
> >
> >>then it should be possible to eliminate a system call by not doing the
> >>initial read before the select.
> >>
> Here's a patch that disables the read before the select, but only for
> the HTTP client socket.

hey brian, i think i prefer the approach below instead.  but i haven't
been able to test this patch because my top-of-tree httpd-2.0 won't build
with top-of-tree apr/apr-util.

the approach i'm using doesn't require any changes to any applications --
it just implements the heuristic i mentioned earlier.  whenever a read()
returns an incomplete buffer assume we want to select() first before
read()ing again (and all of this only if the socket is non-blocking).

-dean

Index: srclib/apr/include/apr_network_io.h
===================================================================
RCS file: /home/cvs/apr/include/apr_network_io.h,v
retrieving revision 1.102
diff -u -r1.102 apr_network_io.h
--- srclib/apr/include/apr_network_io.h	2001/05/02 02:32:44	1.102
+++ srclib/apr/include/apr_network_io.h	2001/07/07 18:18:14
@@ -102,6 +102,12 @@
                                    * APR_TCP_NODELAY should be turned on
                                    * again when NOPUSH is turned off
                                    */
+/* an internal flag.  this is set on non-blocking sockets
+ * (APR_SO_TIMEOUT != 0) on which the previous read() did not fill a
+ * buffer completely.  the next apr_recv() will first call select()/poll()
+ * rather than going straight into read().
+ */
+#define APR_INCOMPLETE_READ 4096

 #define APR_POLLIN    0x001
 #define APR_POLLPRI   0x002
Index: srclib/apr/network_io/unix/sendrecv.c
===================================================================
RCS file: /home/cvs/apr/network_io/unix/sendrecv.c,v
retrieving revision 1.65
diff -u -r1.65 sendrecv.c
--- srclib/apr/network_io/unix/sendrecv.c	2001/05/03 22:38:00	1.65
+++ srclib/apr/network_io/unix/sendrecv.c	2001/07/07 18:18:16
@@ -125,14 +125,21 @@
 apr_status_t apr_recv(apr_socket_t *sock, char *buf, apr_size_t *len)
 {
     ssize_t rv;
-
+    apr_status_t arv;
+
+    if (sock->netmask & APR_INCOMPLETE_READ) {
+	sock->netmask &= ~APR_INCOMPLETE_READ;
+    	goto do_select;
+    }
+
     do {
-        rv = read(sock->socketdes, buf, (*len));
+	    rv = read(sock->socketdes, buf, (*len));
     } while (rv == -1 && errno == EINTR);

     if (rv == -1 && (errno == EAGAIN || errno == EWOULDBLOCK) &&
       sock->timeout != 0) {
-        apr_status_t arv = wait_for_io_or_timeout(sock, 1);
+do_select:
+        arv = wait_for_io_or_timeout(sock, 1);
         if (arv != APR_SUCCESS) {
             *len = 0;
             return arv;
@@ -146,6 +153,9 @@
     if (rv == -1) {
         (*len) = 0;
         return errno;
+    }
+    if (sock->timeout && rv < *len) {
+	sock->netmask |= APR_INCOMPLETE_READ;
     }
     (*len) = rv;
     if (rv == 0) {
Index: srclib/apr/network_io/unix/sockopt.c
===================================================================
RCS file: /home/cvs/apr/network_io/unix/sockopt.c,v
retrieving revision 1.45
diff -u -r1.45 sockopt.c
--- srclib/apr/network_io/unix/sockopt.c	2001/04/05 18:56:08	1.45
+++ srclib/apr/network_io/unix/sockopt.c	2001/07/07 18:18:17
@@ -216,6 +216,12 @@
                 }
             }
         }
+	/* must disable the incomplete read support if we change to a
+	 * blocking socket.
+	 */
+	if (on == 0) {
+	    sock->netmask &= ~APR_INCOMPLETE_READ;
+	}
         sock->timeout = on;
         apr_set_option(&sock->netmask, APR_SO_TIMEOUT, on);
     }


Re: Limit directive and unimplemented methods

Posted by "Roy T. Fielding" <fi...@ebuilt.com>.
> It doesn't logically follow that just because the server has a <limit>
> directive with that method name, it necessarily implements that method.

There is no following to the logic -- it is a definition.

      In Apache httpd, LIMITable == implemented.

The only time 501 is preferred is if the server as a whole, in its entirety,
has never heard of that method before and isn't willing to pass it to
the resource handler to find out if it is allowed.  501 is normally only
sent by proxy servers that have been configured to block unknown methods,
and by old/special-purpose servers that only support a fixed set of methods.

....Roy


Re: Limit directive and unimplemented methods

Posted by Cody Sherr <cs...@covalent.net>.
On Wed, 18 Jul 2001, Roy T. Fielding wrote:

> > I have a question regarding unsupported methods in <LIMIT> directives.
> >
> > Let's say that you could limit arbitrary methods that different modules
> > have implemented. For this example, a module has defined a new method FOO,
> > but no module handles method FOO2:
> >
> > In the cconfig file:
> >
> >
> > <LIMIT FOO FOO2>
> >  ...
> > </LIMIT>
> >
> > What would the correct behavior be if a user made a request with method
> > FOO2? It seems to me that according to the RFC, it should ignore this
> > Limit directive and return 501 (not implemented) instead of 401 (auth
> > required)  or 405 (method not allowed).
>
> If the server has a limit directive with the method name, then it knows
> the method and has therefore implemented it.  The module determines what
> is allowed per resource, not per server, so either 401 or 405 should be
> returned.
>
> ....Roy
>
>

It doesn't logically follow that just because the server has a <limit>
directive with that method name, it necessarily implements that method.

As stated in the example, the server has definitely not implemented method
FOO2, but it has been entered in a LIMIT config directive. To rephrase the
question, what would the correct behavior be for a server responding to
request methods that were LIMITED but not implemented?

It seems to me that 501 is more appropriate.

regards,

-- 
Cody Sherr

Engineer
Covalent Technologies

phone: (415)536-5292
email: csherr@covalent.net




Re: Limit directive and unimplemented methods

Posted by "Roy T. Fielding" <fi...@ebuilt.com>.
> I have a question regarding unsupported methods in <LIMIT> directives.
> 
> Let's say that you could limit arbitrary methods that different modules
> have implemented. For this example, a module has defined a new method FOO,
> but no module handles method FOO2:
> 
> In the cconfig file:
> 
> 
> <LIMIT FOO FOO2>
>  ...
> </LIMIT>
> 
> What would the correct behavior be if a user made a request with method
> FOO2? It seems to me that according to the RFC, it should ignore this
> Limit directive and return 501 (not implemented) instead of 401 (auth
> required)  or 405 (method not allowed).

If the server has a limit directive with the method name, then it knows
the method and has therefore implemented it.  The module determines what
is allowed per resource, not per server, so either 401 or 405 should be
returned.

....Roy


Limit directive and unimplemented methods

Posted by Cody Sherr <cs...@covalent.net>.
I have a question regarding unsupported methods in <LIMIT> directives.

Let's say that you could limit arbitrary methods that different modules
have implemented. For this example, a module has defined a new method FOO,
but no module handles method FOO2:

In the cconfig file:


<LIMIT FOO FOO2>
 ...
</LIMIT>

What would the correct behavior be if a user made a request with method
FOO2? It seems to me that according to the RFC, it should ignore this
Limit directive and return 501 (not implemented) instead of 401 (auth
required)  or 405 (method not allowed).

The relevant part of RFC 2616:

>>> start quote
5.1.1 Method

The Method token indicates the method to be performed on the resource
identified by the Request-URI. The method is case-sensitive.

       Method         = "OPTIONS"                ; Section 9.2
                      | "GET"                    ; Section 9.3
                      | "HEAD"                   ; Section 9.4
                      | "POST"                   ; Section 9.5
                      | "PUT"                    ; Section 9.6
                      | "DELETE"                 ; Section 9.7
                      | "TRACE"                  ; Section 9.8
                      | "CONNECT"                ; Section 9.9
                      | extension-method
       extension-method = token

The list of methods allowed by a resource can be specified in an Allow
header field (section 14.7). The return code of the response always
notifies the client whether a method is currently allowed on a resource,
since the set of allowed methods can change dynamically. An origin server
SHOULD return the status code 405 (Method Not Allowed) if the method is
known by the origin server but not allowed for the requested resource, and
501 (Not Implemented) if the method is unrecognized or not implemented by
the origin server. The methods GET and HEAD MUST be supported by all
general-purpose servers. All other methods are OPTIONAL; however, if the
above methods are implemented, they MUST be implemented with the same
semantics as those specified in section 9.
<<< end quote


Thanks for your time,

-- 
Cody Sherr

Engineer
Covalent Technologies

phone: (415)536-5292
email: csherr@covalent.net



Re: Extraneous socket read?

Posted by Brian Pane <bp...@pacbell.net>.
dean gaudet wrote:

>On Sun, 8 Jul 2001, dean gaudet wrote:
>
>>On Sat, 7 Jul 2001, Bill Stoddard wrote:
>>
>>>Humm... If you use TCP_DEFER_ACCEPT, how to you handle clients
>>>connecting but not sending any bytes?
>>>
>>i'm not sure what happens if they connect and close before the timeout.
>>
>
>the socket is passed to accept() and you read() an EOF immediately... just
>like we'd like to see.
>
>also, it appears the timeout is broken (or i'm using it wrong)... awaiting
>word from kernel gurus.
>
Any good news on TCP_DEFER_ACCEPT?  If timeout handling is broken
in the kernel, I propose that it's better to just put in a
select-before-read patch for now, and later retrofit support for
TCP_DEFER_ACCEPT when timeouts work.  (And for Solaris, the
select-before-read solution is probably the long term solution
anyway.)

--Brian



Re: Extraneous socket read?

Posted by Brian Pane <bp...@pacbell.net>.
dean gaudet wrote:

>
>On Sun, 8 Jul 2001, Brian Pane wrote:
>
>>So I guess the ideal algorithm for retrieving the initial request
>>from a client is:
>>
>>  OS with working TCP_DEFER_ACCEPT or equivalent: select first, then read
>>  OS without working TCP_DEFER_ACCEPT or equivalent: read first
>>
>
>isn't it the other way around?
>
Yes; I transposed the rules for the two cases.  What I should have said was:
  OS with working TCP_DEFER_ACCEPT or equivalent: read first
  OS without working TCP_DEFER_ACCEPT or equivalent: select first

--Brian



Re: Extraneous socket read?

Posted by dean gaudet <dg...@arctic.org>.

On Sun, 8 Jul 2001, Brian Pane wrote:

> So I guess the ideal algorithm for retrieving the initial request
> from a client is:
>
>   OS with working TCP_DEFER_ACCEPT or equivalent: select first, then read
>   OS without working TCP_DEFER_ACCEPT or equivalent: read first

isn't it the other way around?

if TCP_DEFER_ACCEPT exists, then you don't get an accept() event until the
first data packet has arrived -- so you can (should) assume that a read()
will succeed.

if TCP_DEFER_ACCEPT doesn't exist, then you're probably (as long as modem
users still exist) going to get EAGAIN on the first read().  but i dunno.
maybe someone wants to hack up a production server with some debugging
code to measure if this happens?

maybe you were right to begin with and i should just set
APR_INCOMPLETE_READ by default so we always try read() first :)

> It's probably worth the complexity to support TCP_DEFER_ACCEPT on
> platforms where it's available.  My rationale for this is that if
> some idiot decides to open a thousand connections to your httpd and
> never send anything, it's more lightweight to buffer each one in
> a node in a kernel list while awaiting a timeout than to buffer it
> in a connection_rec in the httpd.

oh yeah it's definitely a win.  i'm just not sure what the APR API should
be given that freebsd and linux's implementations differ so much.  it
should also be a run time config option.

-dean


Re: Extraneous socket read?

Posted by Brian Pane <bp...@pacbell.net>.
dean gaudet wrote:

>On Sun, 8 Jul 2001, dean gaudet wrote:
>
>>On Sat, 7 Jul 2001, Bill Stoddard wrote:
>>
>>>Humm... If you use TCP_DEFER_ACCEPT, how to you handle clients
>>>connecting but not sending any bytes?
>>>
>>i'm not sure what happens if they connect and close before the timeout.
>>
>
>the socket is passed to accept() and you read() an EOF immediately... just
>like we'd like to see.
>
>also, it appears the timeout is broken (or i'm using it wrong)... awaiting
>word from kernel gurus.
>
So I guess the ideal algorithm for retrieving the initial request
from a client is:

  OS with working TCP_DEFER_ACCEPT or equivalent: select first, then read
  OS without working TCP_DEFER_ACCEPT or equivalent: read first

It's probably worth the complexity to support TCP_DEFER_ACCEPT on
platforms where it's available.  My rationale for this is that if
some idiot decides to open a thousand connections to your httpd and
never send anything, it's more lightweight to buffer each one in
a node in a kernel list while awaiting a timeout than to buffer it
in a connection_rec in the httpd.

--Brian




Re: Extraneous socket read?

Posted by dean gaudet <dg...@arctic.org>.
On Sun, 8 Jul 2001, dean gaudet wrote:

> On Sat, 7 Jul 2001, Bill Stoddard wrote:
>
> > Humm... If you use TCP_DEFER_ACCEPT, how to you handle clients
> > connecting but not sending any bytes?
>
> i'm not sure what happens if they connect and close before the timeout.

the socket is passed to accept() and you read() an EOF immediately... just
like we'd like to see.

also, it appears the timeout is broken (or i'm using it wrong)... awaiting
word from kernel gurus.

-dean


Re: Extraneous socket read?

Posted by dean gaudet <dg...@arctic.org>.

On Sat, 7 Jul 2001, Bill Stoddard wrote:

>
> > On Sat, 7 Jul 2001, Brian Pane wrote:
> >
> > > If I'm reading the code right, there's one problem with this approach: the
> > > APR_INCOMPLETE_READ flag doesn't get set until after the first
> > > read on a socket, so the first read on a new connection (the one that
> > > usually
> > > returns EAGAIN) doesn't get skipped like it should.  I _think_ the solution
> > > is to set APR_INCOMPLETE_READ on any newly created socket, unless
> > > you can think of cases where that would break something else.
> >
> > oh right.
> >
> > plus there's one more thing to take into account:  FreeBSD's
> > SO_ACCEPTFILTER and linux's TCP_DEFER_ACCEPT ... both of which won't
> > present a socket for accept until there's some data to be read.
>
> Humm... If you use TCP_DEFER_ACCEPT, how to you handle clients
> connecting but not sending any bytes?

i'm not sure what happens if they connect and close before the timeout.

there is a timeout though, so if they connect and sit around you'll get
the socket eventually.  i don't think there's any indication that it timed
out -- we'd probably want to set a really low timeout like 2 seconds, then
we can just continue with our usual timeout on top of that and it'll be
close enough.

i'm not sure there's a timeout in the freebsd SO_ACCEPTFILTER ... anyone
with freebsd foo want to comment on these cases?

-dean


Re: Extraneous socket read?

Posted by Bill Stoddard <bi...@wstoddard.com>.
> On Sat, 7 Jul 2001, Brian Pane wrote:
>
> > If I'm reading the code right, there's one problem with this approach: the
> > APR_INCOMPLETE_READ flag doesn't get set until after the first
> > read on a socket, so the first read on a new connection (the one that
> > usually
> > returns EAGAIN) doesn't get skipped like it should.  I _think_ the solution
> > is to set APR_INCOMPLETE_READ on any newly created socket, unless
> > you can think of cases where that would break something else.
>
> oh right.
>
> plus there's one more thing to take into account:  FreeBSD's
> SO_ACCEPTFILTER and linux's TCP_DEFER_ACCEPT ... both of which won't
> present a socket for accept until there's some data to be read.

Humm... If you use TCP_DEFER_ACCEPT, how to you handle clients connecting but not sending
any bytes?

Bill


[PATCH] Re: Extraneous socket read?

Posted by Brian Pane <bp...@pacbell.net>.
Bill Stoddard wrote:

>Brian,
>Are you still considering working on an APR_INCOMPLETE_READ/APR_OPTIMISTIC_READ socket
>option?  I think something like this will be useful to the lingering close code
>(ap_lingering_close).  During lingering close, we do a non-blocking read first which
>returns EAGAIN then do a select.  In this case, it is clearly a win to just do the select
>(since the common case will be that we have no bytes to read on the inbound side of the
>connection) and avoid the initial read altogether.
>
>I think I prefer the existing operation (read then select) for the default but would like
>a way to tell APR to do a select then a read for special cases like this.
>
>Bill
>
Here's a patch that does the select before the read in three cases:
  * First read on a newly accepted connection
  * First read  during lingering close
  * Any read on socket where the last read returned fewer bytes than
    were requested.
The 3rd case is handled by default in the APR socket routines, while
the first two are controlled by the application (i.e., the httpd
overrides the default read-before-select behavior in these two
specific cases where it's useful to force select-first).  If anybody
later adds support for TCP_DEFER_ACCEPT, we can disable the select-first
on the initial read.

Half of the code in this patch is taken from the patch that Dean
posted; his heuristic for handling incomplete reads is more robust
than my original code.

--Brian

Index: srclib/apr/include/apr_network_io.h
===================================================================
RCS file: /home/cvspublic/apr/include/apr_network_io.h,v
retrieving revision 1.107
diff -u -r1.107 apr_network_io.h
--- srclib/apr/include/apr_network_io.h    2001/07/26 15:39:25    1.107
+++ srclib/apr/include/apr_network_io.h    2001/07/27 23:47:50
@@ -103,6 +103,17 @@
                                    * APR_TCP_NODELAY should be turned on
                                    * again when NOPUSH is turned off
                                    */
+#define APR_INCOMPLETE_READ 4096  /* Set on non-blocking sockets
+                   * (APR_SO_TIMEOUT != 0) on which the
+                   * previous read() did not fill a buffer
+                   * completely.  the next apr_recv() will
+                   * first call select()/poll() rather than
+                   * going straight into read().  (Can also
+                   * be set by an application to force a
+                   * select()/poll() call before the next
+                   * read, in cases where the app expects
+                   * that an immediate read would fail.)
+                   */
 
 #define APR_POLLIN    0x001
 #define APR_POLLPRI   0x002
Index: srclib/apr/network_io/unix/sockopt.c
===================================================================
RCS file: /home/cvspublic/apr/network_io/unix/sockopt.c,v
retrieving revision 1.47
diff -u -r1.47 sockopt.c
--- srclib/apr/network_io/unix/sockopt.c    2001/07/25 18:01:01    1.47
+++ srclib/apr/network_io/unix/sockopt.c    2001/07/27 23:47:50
@@ -203,6 +203,12 @@
                 }
             }
         }
+    /* must disable the incomplete read support if we change to a
+     * blocking socket.
+     */
+    if (on == 0) {
+        sock->netmask &= ~APR_INCOMPLETE_READ;
+    }
         sock->timeout = on;
         apr_set_option(&sock->netmask, APR_SO_TIMEOUT, on);
     }
@@ -265,6 +271,9 @@
 #else
         return APR_ENOTIMPL;
 #endif
+    }
+    if (opt & APR_INCOMPLETE_READ) {
+    apr_set_option(&sock->netmask, APR_INCOMPLETE_READ, on);
     }
 
     return APR_SUCCESS;
Index: srclib/apr/network_io/unix/sendrecv.c
===================================================================
RCS file: /home/cvspublic/apr/network_io/unix/sendrecv.c,v
retrieving revision 1.74
diff -u -r1.74 sendrecv.c
--- srclib/apr/network_io/unix/sendrecv.c    2001/07/26 17:11:04    1.74
+++ srclib/apr/network_io/unix/sendrecv.c    2001/07/27 23:47:50
@@ -125,14 +125,21 @@
 apr_status_t apr_recv(apr_socket_t *sock, char *buf, apr_size_t *len)
 {
     ssize_t rv;
-   
+    apr_status_t arv;
+
+    if (sock->netmask & APR_INCOMPLETE_READ) {
+    sock->netmask &= ~APR_INCOMPLETE_READ;
+        goto do_select;
+    }
+
     do {
         rv = read(sock->socketdes, buf, (*len));
     } while (rv == -1 && errno == EINTR);
 
     if (rv == -1 && (errno == EAGAIN || errno == EWOULDBLOCK) &&
       sock->timeout != 0) {
-        apr_status_t arv = apr_wait_for_io_or_timeout(sock, 1);
+do_select:
+    arv = apr_wait_for_io_or_timeout(sock, 1);
         if (arv != APR_SUCCESS) {
             *len = 0;
             return arv;
@@ -146,6 +153,9 @@
     if (rv == -1) {
         (*len) = 0;
         return errno;
+    }
+    if (sock->timeout && rv < *len) {
+    sock->netmask |= APR_INCOMPLETE_READ;
     }
     (*len) = rv;
     if (rv == 0) {
Index: server/protocol.c
===================================================================
RCS file: /home/cvspublic/httpd-2.0/server/protocol.c,v
retrieving revision 1.33
diff -u -r1.33 protocol.c
--- server/protocol.c    2001/07/24 22:55:29    1.33
+++ server/protocol.c    2001/07/27 23:47:51
@@ -588,6 +588,12 @@
                      (int)(keptalive
                      ? r->server->keep_alive_timeout * APR_USEC_PER_SEC
                      : r->server->timeout * APR_USEC_PER_SEC));
+
+    /* Do a select before the first read, rather than trying the
+     * read first (this is based on empirical observations that
+     * calling read first typically results in EAGAIN/EWOULDBLOCK)
+     */
+    apr_setsocketopt(conn->client_socket, APR_INCOMPLETE_READ, 1);
                     
     /* Get the request... */
     if (!read_request_line(r)) {
Index: server/connection.c
===================================================================
RCS file: /home/cvspublic/httpd-2.0/server/connection.c,v
retrieving revision 1.83
diff -u -r1.83 connection.c
--- server/connection.c    2001/07/12 03:20:48    1.83
+++ server/connection.c    2001/07/27 23:47:51
@@ -195,6 +195,7 @@
      */
     timeout = SECONDS_TO_LINGER * APR_USEC_PER_SEC;
     apr_setsocketopt(c->client_socket, APR_SO_TIMEOUT, timeout);
+    apr_setsocketopt(c->client_socket, APR_INCOMPLETE_READ, 1);
     for (;;) {
         nbytes = sizeof(dummybuf);
         rc = apr_recv(c->client_socket, dummybuf, &nbytes);



Re: Extraneous socket read?

Posted by Brian Pane <bp...@pacbell.net>.
Bill Stoddard wrote:

>Brian,
>Are you still considering working on an APR_INCOMPLETE_READ/APR_OPTIMISTIC_READ socket
>option?  I think something like this will be useful to the lingering close code
>(ap_lingering_close).  During lingering close, we do a non-blocking read first which
>returns EAGAIN then do a select.  In this case, it is clearly a win to just do the select
>(since the common case will be that we have no bytes to read on the inbound side of the
>connection) and avoid the initial read altogether.
>
>I think I prefer the existing operation (read then select) for the default but would like
>a way to tell APR to do a select then a read for special cases like this.
>
I think the most general solution is to adopt Dean's APR_INCOMPLETE_READ
logic and let apps set the flag themselves for special cases like the first
read and the lingering close.  I'll try to post a patch this afternoon that
adds the first-read and lingering-close optimizations on top of Dean's code.

--Brian





Re: Extraneous socket read?

Posted by Bill Stoddard <bi...@wstoddard.com>.
Brian,
Are you still considering working on an APR_INCOMPLETE_READ/APR_OPTIMISTIC_READ socket
option?  I think something like this will be useful to the lingering close code
(ap_lingering_close).  During lingering close, we do a non-blocking read first which
returns EAGAIN then do a select.  In this case, it is clearly a win to just do the select
(since the common case will be that we have no bytes to read on the inbound side of the
connection) and avoid the initial read altogether.

I think I prefer the existing operation (read then select) for the default but would like
a way to tell APR to do a select then a read for special cases like this.

Bill

> On Sat, 7 Jul 2001, Brian Pane wrote:
>
> > If I'm reading the code right, there's one problem with this approach: the
> > APR_INCOMPLETE_READ flag doesn't get set until after the first
> > read on a socket, so the first read on a new connection (the one that
> > usually
> > returns EAGAIN) doesn't get skipped like it should.  I _think_ the solution
> > is to set APR_INCOMPLETE_READ on any newly created socket, unless
> > you can think of cases where that would break something else.
>
> oh right.
>
> plus there's one more thing to take into account:  FreeBSD's
> SO_ACCEPTFILTER and linux's TCP_DEFER_ACCEPT ... both of which won't
> present a socket for accept until there's some data to be read.
>
> hmm.  we don't have TCP_DEFER_ACCEPT support yet.
>
> -dean
>
>
>
>


Re: Extraneous socket read?

Posted by dean gaudet <de...@arctic.org>.
On Sat, 7 Jul 2001, Brian Pane wrote:

> If I'm reading the code right, there's one problem with this approach: the
> APR_INCOMPLETE_READ flag doesn't get set until after the first
> read on a socket, so the first read on a new connection (the one that
> usually
> returns EAGAIN) doesn't get skipped like it should.  I _think_ the solution
> is to set APR_INCOMPLETE_READ on any newly created socket, unless
> you can think of cases where that would break something else.

oh right.

plus there's one more thing to take into account:  FreeBSD's
SO_ACCEPTFILTER and linux's TCP_DEFER_ACCEPT ... both of which won't
present a socket for accept until there's some data to be read.

hmm.  we don't have TCP_DEFER_ACCEPT support yet.

-dean





Re: Extraneous socket read?

Posted by dean gaudet <de...@arctic.org>.
On Sat, 7 Jul 2001, Brian Pane wrote:

> If I'm reading the code right, there's one problem with this approach: the
> APR_INCOMPLETE_READ flag doesn't get set until after the first
> read on a socket, so the first read on a new connection (the one that
> usually
> returns EAGAIN) doesn't get skipped like it should.  I _think_ the solution
> is to set APR_INCOMPLETE_READ on any newly created socket, unless
> you can think of cases where that would break something else.

oh right.

plus there's one more thing to take into account:  FreeBSD's
SO_ACCEPTFILTER and linux's TCP_DEFER_ACCEPT ... both of which won't
present a socket for accept until there's some data to be read.

hmm.  we don't have TCP_DEFER_ACCEPT support yet.

-dean





Re: Extraneous socket read?

Posted by Brian Pane <bp...@pacbell.net>.
dean gaudet wrote:

>On Mon, 2 Jul 2001, Brian Pane wrote:
>
>>>On Mon, 2 Jul 2001, Brian Pane wrote:
>>>
>>>>then it should be possible to eliminate a system call by not doing the
>>>>initial read before the select.
>>>>
>>Here's a patch that disables the read before the select, but only for
>>the HTTP client socket.
>>
>
>hey brian, i think i prefer the approach below instead.  but i haven't
>been able to test this patch because my top-of-tree httpd-2.0 won't build
>with top-of-tree apr/apr-util.
>
>the approach i'm using doesn't require any changes to any applications --
>it just implements the heuristic i mentioned earlier.  whenever a read()
>returns an incomplete buffer assume we want to select() first before
>read()ing again (and all of this only if the socket is non-blocking).
>

If I'm reading the code right, there's one problem with this approach: the
APR_INCOMPLETE_READ flag doesn't get set until after the first
read on a socket, so the first read on a new connection (the one that 
usually
returns EAGAIN) doesn't get skipped like it should.  I _think_ the solution
is to set APR_INCOMPLETE_READ on any newly created socket, unless
you can think of cases where that would break something else.

--Brian



Re: Extraneous socket read?

Posted by Brian Pane <bp...@pacbell.net>.
dean gaudet wrote:

>On Mon, 2 Jul 2001, Brian Pane wrote:
>
>>>On Mon, 2 Jul 2001, Brian Pane wrote:
>>>
>>>>then it should be possible to eliminate a system call by not doing the
>>>>initial read before the select.
>>>>
>>Here's a patch that disables the read before the select, but only for
>>the HTTP client socket.
>>
>
>hey brian, i think i prefer the approach below instead.  but i haven't
>been able to test this patch because my top-of-tree httpd-2.0 won't build
>with top-of-tree apr/apr-util.
>
>the approach i'm using doesn't require any changes to any applications --
>it just implements the heuristic i mentioned earlier.  whenever a read()
>returns an incomplete buffer assume we want to select() first before
>read()ing again (and all of this only if the socket is non-blocking).
>

If I'm reading the code right, there's one problem with this approach: the
APR_INCOMPLETE_READ flag doesn't get set until after the first
read on a socket, so the first read on a new connection (the one that 
usually
returns EAGAIN) doesn't get skipped like it should.  I _think_ the solution
is to set APR_INCOMPLETE_READ on any newly created socket, unless
you can think of cases where that would break something else.

--Brian