Re: [naviserver-devel] naviserver/nsd driver.c,1.119,1.120

[Stephen D. <sd...@gm...>]

On Wed, Oct 22, 2008 at 4:23 AM, Vlad Seryakov
<ser...@us...> wrote:
> Update of /cvsroot/naviserver/naviserver/nsd
> In directory fdv4jf1.ch3.sourceforge.com:/tmp/cvs-serv22913/nsd
>
> Modified Files:
>        driver.c
> Log Message:
> new accept status, SSL driver works now
>
>
> Index: driver.c
> ===================================================================
> RCS file: /cvsroot/naviserver/naviserver/nsd/driver.c,v
> retrieving revision 1.119
> retrieving revision 1.120
> diff -C2 -d -r1.119 -r1.120
> *** driver.c    20 Oct 2008 00:06:54 -0000      1.119
> --- driver.c    22 Oct 2008 03:23:06 -0000      1.120
> ***************
> *** 787,791 ****
>   * Results:
>   *      _ACCEPT:       a socket was accepted, poll for data
> !  *      _ACCEPT_DATA:  a socket was accepted, data present
>   *      _ACCEPT_ERROR: no socket was accepted
>   *
> --- 787,793 ----
>   * Results:
>   *      _ACCEPT:       a socket was accepted, poll for data
> !  *      _ACCEPT_DATA:  a socket was accepted, data present, read immediately
> !  *                     if in async mode, defer reading to connection thread
> !  *      _ACCEPT_QUEUE: a socket was accepted, queue immediately
>   *      _ACCEPT_ERROR: no socket was accepted
>   *
> ***************
> *** 1523,1534 ****
>
>      } else {
>          drvPtr->queuesize++;
>
> -         /*
> -          * If there is already data present then read it without
> -          * polling if we're in async mode.
> -          */
> -
>          if (status == NS_DRIVER_ACCEPT_DATA) {
>              if (drvPtr->opts & NS_DRIVER_ASYNC) {
>                  status = SockRead(sockPtr, 0);
> --- 1525,1538 ----
>
>      } else {
> +         status = SOCK_MORE;
>          drvPtr->queuesize++;
>
>          if (status == NS_DRIVER_ACCEPT_DATA) {
> +
> +             /*
> +              * If there is already data present then read it without
> +              * polling if we're in async mode.
> +              */
> +
>              if (drvPtr->opts & NS_DRIVER_ASYNC) {
>                  status = SockRead(sockPtr, 0);
> ***************
> *** 1541,1553 ****
>
>                  /*
> !                  *  We need to call this to make sure socket has request structure allocated,
> !                  *  otherwise NsGetRequest will call SockRead which is not what this driver wants
>                   */
>
> -                 SockPrepare(sockPtr);
>                  status = SOCK_READY;
>              }
> !         } else {
> !             status = SOCK_MORE;
>          }
>      }
> --- 1545,1564 ----
>
>                  /*
> !                  * Queue this socket without reading, NsGetRequest in
> !                  * the connection thread will perform actual reading of the request
>                   */
>
>                  status = SOCK_READY;
>              }
> !         } else
> !         if (status == NS_DRIVER_ACCEPT_QUEUE) {
> !
> !             /*
> !              *  We need to call SockPrepare to make sure socket has request structure allocated,
> !              *  otherwise NsGetRequest will call SockRead which is not what this driver wants
> !              */
> !
> !             SockPrepare(sockPtr);
> !             status = SOCK_READY;
>          }
>      }


Why the new accept status: NS_DRIVER_ACCEPT_QUEUE ?

The idea we talked about with the driver callbacks was that a
non-standard driver could fake-up a request in it's read callback. So,
whereas at the mo you have something like this:

modules/nsudp/nsudp.c:

static NS_DRIVER_ACCEPT_STATUS
Accept(Ns_Sock *sock, SOCKET listensock, struct sockaddr *sockaddrPtr,
int *socklenPtr)
{
    sock->sock = listensock;
    return NS_DRIVER_ACCEPT_DATA;
}

static ssize_t
Recv(Ns_Sock *sock, struct iovec *bufs, int nbufs, Ns_Time
*timeoutPtr, int flags)
{
     socklen_t size = sizeof(struct sockaddr_in);

     return recvfrom(sock->sock, bufs->iov_base, bufs->iov_len, 0,
(struct sockaddr*)&sock->sa, &size);
}


You would instead do something like this:


static ssize_t
Recv(Ns_Sock *sock, struct iovec *bufs, int nbufs, Ns_Time
*timeoutPtr, int flags)
{
    static const char request[] = "GET /whateva HTTP/1.0\r\n\r\n";
    size_t requestLen = sizeof(request);
    socklen_t sockLen = sizeof(struct sockaddr_in);

    memcpy(bufs->iov_base, request, requestLen);

    return recvfrom(sock->sock,
                    bufs->iov_base + requestLen,
                    bufs->iov_len - requestLen,
                    0, (struct sockaddr*) &sock->sa, &size);
}

(checking for buffer lengths skipped here)


The advantage of doing it this way is that everything is much simpler
from the driver threads point of view. It doesn't need to know
anything special about non-standard protocols, and there isn't
anything in the driver callback api that isn't also useful to the HTTP
server.


Also, this can't work:

nsd/driver.c:1515

status = DriverAccept(sockPtr);

if (status == NS_DRIVER_ACCEPT_ERROR) {
    status = SOCK_ERROR;
    ...

} else {
    status = SOCK_MORE;
    ...

    if (status == NS_DRIVER_ACCEPT_DATA) {
        ...
    } else if (status == NS_DRIVER_ACCEPT_QUEUE) {
        ...
    }
}