naviserver-devel

Re: [naviserver-devel] [naviserver-commits] naviserver/nsd driver.c, 1.97, 1.98

[Vlad S. <vl...@cr...>]

I think it was working perfectly fine, new buffer size is set on 
accepted socket, new one, not the one we listened.


I used to play with these options for one of the modules i wrote and at 
least on Linux it was working fine.

Also i found this about this option:
You can increase a stream socket's buffer size at any time, but decrease 
it only prior to establishing a connection. The maximum buffer size for 
stream sockets is 262144 bytes.



Zoran Vasiljevic wrote:
> Update of /cvsroot/naviserver/naviserver/nsd
> In directory sc8-pr-cvs16.sourceforge.net:/tmp/cvs-serv8949/nsd
> 
> Modified Files:
> 	driver.c 
> Log Message:
> nsd/driver.c: Socket send/recv buffer setup moved to NsStartDrivers()
> and commented as real NO-OP, as too late in the startup sequence.
> TCP honours the buffer-size setup prior to listen() or connect() only.
> Without significant API plumbing, the per-driver values for send and
> receive buffers are just useless.
> 
> 
> Index: driver.c
> ===================================================================
> RCS file: /cvsroot/naviserver/naviserver/nsd/driver.c,v
> retrieving revision 1.97
> retrieving revision 1.98
> diff -C2 -d -r1.97 -r1.98
> *** driver.c	18 Dec 2007 18:21:39 -0000	1.97
> --- driver.c	26 Apr 2008 19:47:52 -0000	1.98
> ***************
> *** 551,627 ****
>   NsStartDrivers(void)
>   {
> !     Driver *drvPtr;
> !     SpoolerQueue *queuePtr;
>   
>       /*
> !      * Listen on all drivers.
>        */
>   
>       drvPtr = firstDrvPtr;
>   
>       while (drvPtr != NULL) {
> -         if (drvPtr->opts & NS_DRIVER_UNIX) {
> -             drvPtr->sock = Ns_SockListenUnix(drvPtr->bindaddr,
> -                                              drvPtr->opts & NS_DRIVER_UDP ? 0 :
> -                                                             drvPtr->backlog, 0);
>   
> !         } else if (drvPtr->opts & NS_DRIVER_UDP) {
> !             drvPtr->sock = Ns_SockListenUdp(drvPtr->bindaddr,
> !                                             drvPtr->port);
>   
>           } else {
> !             drvPtr->sock = Ns_SockListenEx(drvPtr->bindaddr,
> !                                            drvPtr->port, drvPtr->backlog);
>           }
>           if (drvPtr->sock == INVALID_SOCKET) {
> !             Ns_Log(Error, "%s: failed to listen on %s:%d: %s",
> !                    drvPtr->name, drvPtr->address, drvPtr->port,
> !                    ns_sockstrerror(ns_sockerrno));
>           } else {
>   
>               Ns_SockSetNonBlocking(drvPtr->sock);
> -             Ns_Log(Notice, "%s: listening on %s:%d",
> -                    drvPtr->name, drvPtr->address, drvPtr->port);
>   
>               /*
> !              * Create the spooler thread(s).
>                */
>   
> !             queuePtr = drvPtr->spooler.firstPtr;
> !             while (queuePtr) {
> !                 if (ns_sockpair(queuePtr->pipe) != 0) {
> !                     Ns_Fatal("driver: ns_sockpair() failed: %s",
> !                              ns_sockstrerror(ns_sockerrno));
> !                 }
> !                 Ns_ThreadCreate(SpoolerThread, queuePtr, 0,
> !                                 &queuePtr->thread);
> !                 queuePtr = queuePtr->nextPtr;
>               }
>   
>               /*
> !              * Create the writer thread(s)
>                */
>   
> !             queuePtr = drvPtr->writer.firstPtr;
> !             while (queuePtr) {
> !                 if (ns_sockpair(queuePtr->pipe) != 0) {
> !                     Ns_Fatal("driver: ns_sockpair() failed: %s",
> !                              ns_sockstrerror(ns_sockerrno));
>                   }
> -                 Ns_ThreadCreate(WriterThread, queuePtr, 0,
> -                                 &queuePtr->thread);
> -                 queuePtr = queuePtr->nextPtr;
>               }
>           }
>           drvPtr = drvPtr->nextPtr;
>       }
>   
>       /*
> !      * Create the driver thread.
>        */
>   
> !     if (firstDrvPtr != NULL) {
> !         if (ns_sockpair(drvPipe) != 0) {
> !             Ns_Fatal("driver: ns_sockpair() failed: %s",
>                        ns_sockstrerror(ns_sockerrno));
>           }
> --- 551,648 ----
>   NsStartDrivers(void)
>   {
> !     int          i, bl, ncommd;
> !     Driver       *drvPtr;
> !     SpoolerQueue *qPtr;
> ! 
> !     struct {
> !         Ns_ThreadProc *tproc;
> !         SpoolerQueue  *queue;
> !     } thr[] = {{SpoolerThread, NULL}, {WriterThread,  NULL}};
>   
>       /*
> !      * Listen on all communication drivers.
>        */
>   
>       drvPtr = firstDrvPtr;
> +     ncommd = 0;
>   
>       while (drvPtr != NULL) {
>   
> !         bl = (drvPtr->opts & NS_DRIVER_UDP) ? 0 : drvPtr->backlog;
>   
> +         if (drvPtr->opts & NS_DRIVER_UNIX) {
> +             drvPtr->sock = Ns_SockListenUnix(drvPtr->bindaddr, bl, 0);
> +         } else if (drvPtr->opts & NS_DRIVER_UDP) {
> +             drvPtr->sock = Ns_SockListenUdp(drvPtr->bindaddr, drvPtr->port);
>           } else {
> !             drvPtr->sock = Ns_SockListenEx(drvPtr->bindaddr, drvPtr->port, bl);
>           }
>           if (drvPtr->sock == INVALID_SOCKET) {
> !             Ns_Log(Error, "%s: failed to listen on %s:%d: %s", drvPtr->name,
> !                    drvPtr->address, drvPtr->port, ns_sockstrerror(ns_sockerrno));
>           } else {
>   
>               Ns_SockSetNonBlocking(drvPtr->sock);
>   
>               /*
> !              * Set the send/recv socket bufsizes if required.
> !              * Note: it's too late to set them here, as TCP manual says:
> !              *
> !              *     On individual connections, the socket buffer size 
> !              *     must be set prior to the listen() or connect() calls
> !              *     in order to have it take effect.
> !              *
> !              * So now what? The binder process already did all of that for
> !              * us early on startup! So whatever we do here below does not
> !              * really matter.  We must move this early in the binding process
> !              * somehow, but that requires significant API plumbing... Phew.
> !              *
>                */
>   
> !             if (drvPtr->sndbuf > 0) {
> !                 setsockopt(drvPtr->sock, SOL_SOCKET, SO_SNDBUF,
> !                            (char *) &drvPtr->sndbuf, sizeof(drvPtr->sndbuf));
> !             }
> !             if (drvPtr->rcvbuf > 0) {
> !                 setsockopt(drvPtr->sock, SOL_SOCKET, SO_RCVBUF,
> !                            (char *) &drvPtr->rcvbuf, sizeof(drvPtr->rcvbuf));
>               }
>   
> +             Ns_Log(Notice, "%s: listening on %s:%d", drvPtr->name,
> +                    drvPtr->address, drvPtr->port);
> + 
> +             ncommd++; /* Yet another successfuly started comm driver */
> + 
>               /*
> !              * Create the spooler/writer thread(s).
>                */
>   
> !             thr[0].queue = drvPtr->spooler.firstPtr;
> !             thr[1].queue = drvPtr->writer.firstPtr;
> ! 
> !             for (i = 0; i < 2; i++) {
> !                 qPtr = thr[i].queue;
> !                 while (qPtr != NULL) {
> !                     if (ns_sockpair(qPtr->pipe)) {
> !                         Ns_Fatal("ns_sockpair() failed: %s",
> !                                  ns_sockstrerror(ns_sockerrno));
> !                     }
> !                     Ns_ThreadCreate(thr[i].tproc, qPtr, 0, &qPtr->thread);
> !                     qPtr = qPtr->nextPtr;
>                   }
>               }
>           }
> + 
>           drvPtr = drvPtr->nextPtr;
>       }
>   
>       /*
> !      * Create the driver thread itself if we managed to
> !      * get at least one of the communication drivers.
>        */
>   
> !     if (ncommd > 0) {
> !         if (ns_sockpair(drvPipe)) {
> !             Ns_Fatal("ns_sockpair() failed: %s",
>                        ns_sockstrerror(ns_sockerrno));
>           }
> ***************
> *** 1678,1694 ****
>       Ns_SockSetNonBlocking(sockPtr->sock);
>   
> -     /*
> -      * Set the send/recv socket bufsizes if required.
> -      */
> - 
> -     if (drvPtr->sndbuf > 0) {
> -         setsockopt(sockPtr->sock, SOL_SOCKET, SO_SNDBUF,
> -                    (char *) &drvPtr->sndbuf, sizeof(drvPtr->sndbuf));
> -     }
> -     if (drvPtr->rcvbuf > 0) {
> -         setsockopt(sockPtr->sock, SOL_SOCKET, SO_RCVBUF,
> -                    (char *) &drvPtr->rcvbuf, sizeof(drvPtr->rcvbuf));
> -     }
> - 
>       drvPtr->queuesize++;
>   
> --- 1699,1702 ----
> 
> 
> -------------------------------------------------------------------------
> This SF.net email is sponsored by the 2008 JavaOne(SM) Conference 
> Don't miss this year's exciting event. There's still time to save $100. 
> Use priority code J8TL2D2. 
> http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone
> _______________________________________________
> naviserver-commits mailing list
> nav...@li...
> https://lists.sourceforge.net/lists/listinfo/naviserver-commits
> 

-- 
Vlad Seryakov
vl...@cr...
http://www.crystalballinc.com/vlad/

Re: [naviserver-devel] [naviserver-commits] naviserver/nsd driver.c, 1.97, 1.98

[Vasiljevic Z. <zv...@ar...>]

On 26.04.2008, at 23:46, Vlad Seryakov wrote:

> I think it was working perfectly fine, new buffer size is set on
> accepted socket, new one, not the one we listened.
>
>
> I used to play with these options for one of the modules i wrote and  
> at
> least on Linux it was working fine.

OK. I will back this off. I was unable to get that working
on my platforms. So I thought to comment this and move
close to the place where it *might* work.

Good to know that this works on Linux at least...

Zoran

Re: [naviserver-devel] [naviserver-commits] naviserver/nsd driver.c, 1.97, 1.98

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

On Sun, Apr 27, 2008 at 9:39 AM, Vasiljevic Zoran <zv...@ar...> wrote:
>
> On 26.04.2008, at 23:46, Vlad Seryakov wrote:
>
>> I think it was working perfectly fine, new buffer size is set on
>> accepted socket, new one, not the one we listened.
>>
>>
>> I used to play with these options for one of the modules i wrote and
>> at
>> least on Linux it was working fine.
>
> OK. I will back this off. I was unable to get that working
> on my platforms. So I thought to comment this and move
> close to the place where it *might* work.
>
> Good to know that this works on Linux at least...


These options can probably just be dropped:

    http://www.psc.edu/networking/projects/tcptune/#options


For Linux it says:

NB: Recent versions of Linux (version 2.6.17 and later) have full
autotuning with 4 MB maximum buffer sizes. Except in some rare cases,
manual tuning is unlikely to substantially improve the performance of
these kernels over most network paths, and is not generally
recommended


This wasn't the web page I was looking for. There's one somewhere that
describes a project that looked at autotuning TCP buffer sizes and I
remember it said that all major OS' were fixed a couple of years ago,
as a result.

I think this is not needed anymore, and probably just makes things
worse if used.

One less knob!

Re: [naviserver-devel] [naviserver-commits] naviserver/nsd driver.c, 1.97, 1.98

[Vasiljevic Z. <zv...@ar...>]

On 11.05.2008, at 20:33, Stephen Deasey wrote:

>
> I think this is not needed anymore, and probably just makes things
> worse if used.
>
> One less knob!

I believe we can ditch that. I never used them. It is a black magic
that works entirely different from OS to OS. And it is really no
gain. If you have a system that needs that kind of tuning, you can
go do that on the kernel level (most OS'es support tweaking IP
options there). So far only a high-speed _and_ high-latency links
would need such tuning.

A word on knobs... I totaly agree with Stephen! LESS is MORE!
The trouble with knobs is they are just an expression of "I do not  
know".
Since we do not know how something will be used, it is easy to
throw a switch or a knob there. But it creates problems with
maintenance AND deployment. Usually MOST of those knobs just
go live their default life. It is our task to define generic
and practicable values on them. Perhaps even auto-tuning options.

So, YES. Rip it out! Who's the volunteer? If no objections I
will have the pleasure...

  
  

Re: [naviserver-devel] [naviserver-commits] naviserver/nsd driver.c, 1.97, 1.98

[Vasiljevic Z. <zv...@ar...>]

On 12.05.2008, at 11:14, Vasiljevic Zoran wrote:

>
> So, YES. Rip it out! Who's the volunteer? If no objections I
> will have the pleasure...

No volunteers, hence I did that. The sndbuf/rcvbuf elements of
the driver struct plus accompanying config code is ripped out.

Cheers
Zoran