Re: [PATCH] dont increase the size of socket buffers in low memory situations

[Alexander Bluhm]

On Tue, Jul 05, 2016 at 03:44:17PM +0200, Simon Mages wrote:
> Thats true, i found also another bug in this diff, the new one follows.

OK bluhm@

> Index: netinet/tcp_usrreq.c
> ===
> RCS file: /cvs/src/sys/netinet/tcp_usrreq.c,v
> retrieving revision 1.131
> diff -u -p -u -p -r1.131 tcp_usrreq.c
> --- netinet/tcp_usrreq.c  18 Jun 2016 10:36:13 -  1.131
> +++ netinet/tcp_usrreq.c  5 Jul 2016 13:41:49 -
> @@ -977,12 +977,13 @@ void
>  tcp_update_sndspace(struct tcpcb *tp)
>  {
>   struct socket *so = tp->t_inpcb->inp_socket;
> - u_long nmax;
> + u_long nmax = so->so_snd.sb_hiwat;
> 
> - if (sbchecklowmem())
> + if (sbchecklowmem()) {
>   /* low on memory try to get rid of some */
> - nmax = tcp_sendspace;
> - else if (so->so_snd.sb_wat != tcp_sendspace)
> + if (tcp_sendspace < nmax)
> + nmax = tcp_sendspace;
> + } else if (so->so_snd.sb_wat != tcp_sendspace)
>   /* user requested buffer size, auto-scaling disabled */
>   nmax = so->so_snd.sb_wat;
>   else
> @@ -1017,10 +1018,11 @@ tcp_update_rcvspace(struct tcpcb *tp)
>   struct socket *so = tp->t_inpcb->inp_socket;
>   u_long nmax = so->so_rcv.sb_hiwat;
> 
> - if (sbchecklowmem())
> + if (sbchecklowmem()) {
>   /* low on memory try to get rid of some */
> - nmax = tcp_recvspace;
> - else if (so->so_rcv.sb_wat != tcp_recvspace)
> + if (tcp_recvspace < nmax)
> + nmax = tcp_recvspace;
> + } else if (so->so_rcv.sb_wat != tcp_recvspace)
>   /* user requested buffer size, auto-scaling disabled */
>   nmax = so->so_rcv.sb_wat;
>   else {

Re: [PATCH] dont increase the size of socket buffers in low memory situations

[Simon Mages]

2016-07-05 15:36 GMT+02:00, Claudio Jeker :
> On Tue, Jul 05, 2016 at 07:22:27AM -0600, Bob Beck wrote:
>> Makes sense to me.  Others?
>>
>>
>> On Tue, Jul 5, 2016 at 4:08 AM, Simon Mages 
>> wrote:
>> > At the moment the buffersize will be set to the default even if the
>> > current value
>> > is smaller.
>> >
>> > The following diff fixes this problem.
>> >
>> > Index: netinet/tcp_usrreq.c
>> > ===
>> > RCS file: /cvs/src/sys/netinet/tcp_usrreq.c,v
>> > retrieving revision 1.131
>> > diff -u -p -u -p -r1.131 tcp_usrreq.c
>> > --- netinet/tcp_usrreq.c18 Jun 2016 10:36:13 -  1.131
>> > +++ netinet/tcp_usrreq.c5 Jul 2016 09:26:24 -
>> > @@ -979,10 +979,11 @@ tcp_update_sndspace(struct tcpcb *tp)
>> > struct socket *so = tp->t_inpcb->inp_socket;
>> > u_long nmax;
>> >
>> > -   if (sbchecklowmem())
>> > +   if (sbchecklowmem()) {
>> > /* low on memory try to get rid of some */
>> > -   nmax = tcp_sendspace;
>> > -   else if (so->so_snd.sb_wat != tcp_sendspace)
>> > +   if (so->so_snd.sb_hiwat < nmax)
>> > +   nmax = tcp_sendspace;
>> > +   } else if (so->so_snd.sb_wat != tcp_sendspace)
>> > /* user requested buffer size, auto-scaling disabled */
>> > nmax = so->so_snd.sb_wat;
>> > else
>
> Here, nmax can be used uninitialized now.
> It needs be initialized to something maybe sb_hiwat?

Thats true, i found also another bug in this diff, the new one follows.

>
>> > @@ -1017,10 +1018,11 @@ tcp_update_rcvspace(struct tcpcb *tp)
>> > struct socket *so = tp->t_inpcb->inp_socket;
>> > u_long nmax = so->so_rcv.sb_hiwat;
>> >
>> > -   if (sbchecklowmem())
>> > +   if (sbchecklowmem()) {
>> > /* low on memory try to get rid of some */
>> > -   nmax = tcp_recvspace;
>> > -   else if (so->so_rcv.sb_wat != tcp_recvspace)
>> > +   if (tcp_recvspace < nmax)
>> > +   nmax = tcp_recvspace;
>> > +   } else if (so->so_rcv.sb_wat != tcp_recvspace)
>> > /* user requested buffer size, auto-scaling disabled */
>> > nmax = so->so_rcv.sb_wat;
>> > else {
>> >
>
> Here there is no issue.
>
> --
> :wq Claudio
>

Index: netinet/tcp_usrreq.c
===
RCS file: /cvs/src/sys/netinet/tcp_usrreq.c,v
retrieving revision 1.131
diff -u -p -u -p -r1.131 tcp_usrreq.c
--- netinet/tcp_usrreq.c18 Jun 2016 10:36:13 -  1.131
+++ netinet/tcp_usrreq.c5 Jul 2016 13:41:49 -
@@ -977,12 +977,13 @@ void
 tcp_update_sndspace(struct tcpcb *tp)
 {
struct socket *so = tp->t_inpcb->inp_socket;
-   u_long nmax;
+   u_long nmax = so->so_snd.sb_hiwat;

-   if (sbchecklowmem())
+   if (sbchecklowmem()) {
/* low on memory try to get rid of some */
-   nmax = tcp_sendspace;
-   else if (so->so_snd.sb_wat != tcp_sendspace)
+   if (tcp_sendspace < nmax)
+   nmax = tcp_sendspace;
+   } else if (so->so_snd.sb_wat != tcp_sendspace)
/* user requested buffer size, auto-scaling disabled */
nmax = so->so_snd.sb_wat;
else
@@ -1017,10 +1018,11 @@ tcp_update_rcvspace(struct tcpcb *tp)
struct socket *so = tp->t_inpcb->inp_socket;
u_long nmax = so->so_rcv.sb_hiwat;

-   if (sbchecklowmem())
+   if (sbchecklowmem()) {
/* low on memory try to get rid of some */
-   nmax = tcp_recvspace;
-   else if (so->so_rcv.sb_wat != tcp_recvspace)
+   if (tcp_recvspace < nmax)
+   nmax = tcp_recvspace;
+   } else if (so->so_rcv.sb_wat != tcp_recvspace)
/* user requested buffer size, auto-scaling disabled */
nmax = so->so_rcv.sb_wat;
else {

Re: [PATCH] dont increase the size of socket buffers in low memory situations

[Claudio Jeker]

On Tue, Jul 05, 2016 at 07:22:27AM -0600, Bob Beck wrote:
> Makes sense to me.  Others?
> 
> 
> On Tue, Jul 5, 2016 at 4:08 AM, Simon Mages  
> wrote:
> > At the moment the buffersize will be set to the default even if the
> > current value
> > is smaller.
> >
> > The following diff fixes this problem.
> >
> > Index: netinet/tcp_usrreq.c
> > ===
> > RCS file: /cvs/src/sys/netinet/tcp_usrreq.c,v
> > retrieving revision 1.131
> > diff -u -p -u -p -r1.131 tcp_usrreq.c
> > --- netinet/tcp_usrreq.c18 Jun 2016 10:36:13 -  1.131
> > +++ netinet/tcp_usrreq.c5 Jul 2016 09:26:24 -
> > @@ -979,10 +979,11 @@ tcp_update_sndspace(struct tcpcb *tp)
> > struct socket *so = tp->t_inpcb->inp_socket;
> > u_long nmax;
> >
> > -   if (sbchecklowmem())
> > +   if (sbchecklowmem()) {
> > /* low on memory try to get rid of some */
> > -   nmax = tcp_sendspace;
> > -   else if (so->so_snd.sb_wat != tcp_sendspace)
> > +   if (so->so_snd.sb_hiwat < nmax)
> > +   nmax = tcp_sendspace;
> > +   } else if (so->so_snd.sb_wat != tcp_sendspace)
> > /* user requested buffer size, auto-scaling disabled */
> > nmax = so->so_snd.sb_wat;
> > else

Here, nmax can be used uninitialized now.
It needs be initialized to something maybe sb_hiwat?

> > @@ -1017,10 +1018,11 @@ tcp_update_rcvspace(struct tcpcb *tp)
> > struct socket *so = tp->t_inpcb->inp_socket;
> > u_long nmax = so->so_rcv.sb_hiwat;
> >
> > -   if (sbchecklowmem())
> > +   if (sbchecklowmem()) {
> > /* low on memory try to get rid of some */
> > -   nmax = tcp_recvspace;
> > -   else if (so->so_rcv.sb_wat != tcp_recvspace)
> > +   if (tcp_recvspace < nmax)
> > +   nmax = tcp_recvspace;
> > +   } else if (so->so_rcv.sb_wat != tcp_recvspace)
> > /* user requested buffer size, auto-scaling disabled */
> > nmax = so->so_rcv.sb_wat;
> > else {
> >

Here there is no issue. 

-- 
:wq Claudio

Re: [PATCH] dont increase the size of socket buffers in low memory situations

[Bob Beck]

Makes sense to me.  Others?


On Tue, Jul 5, 2016 at 4:08 AM, Simon Mages  wrote:
> At the moment the buffersize will be set to the default even if the
> current value
> is smaller.
>
> The following diff fixes this problem.
>
> Index: netinet/tcp_usrreq.c
> ===
> RCS file: /cvs/src/sys/netinet/tcp_usrreq.c,v
> retrieving revision 1.131
> diff -u -p -u -p -r1.131 tcp_usrreq.c
> --- netinet/tcp_usrreq.c18 Jun 2016 10:36:13 -  1.131
> +++ netinet/tcp_usrreq.c5 Jul 2016 09:26:24 -
> @@ -979,10 +979,11 @@ tcp_update_sndspace(struct tcpcb *tp)
> struct socket *so = tp->t_inpcb->inp_socket;
> u_long nmax;
>
> -   if (sbchecklowmem())
> +   if (sbchecklowmem()) {
> /* low on memory try to get rid of some */
> -   nmax = tcp_sendspace;
> -   else if (so->so_snd.sb_wat != tcp_sendspace)
> +   if (so->so_snd.sb_hiwat < nmax)
> +   nmax = tcp_sendspace;
> +   } else if (so->so_snd.sb_wat != tcp_sendspace)
> /* user requested buffer size, auto-scaling disabled */
> nmax = so->so_snd.sb_wat;
> else
> @@ -1017,10 +1018,11 @@ tcp_update_rcvspace(struct tcpcb *tp)
> struct socket *so = tp->t_inpcb->inp_socket;
> u_long nmax = so->so_rcv.sb_hiwat;
>
> -   if (sbchecklowmem())
> +   if (sbchecklowmem()) {
> /* low on memory try to get rid of some */
> -   nmax = tcp_recvspace;
> -   else if (so->so_rcv.sb_wat != tcp_recvspace)
> +   if (tcp_recvspace < nmax)
> +   nmax = tcp_recvspace;
> +   } else if (so->so_rcv.sb_wat != tcp_recvspace)
> /* user requested buffer size, auto-scaling disabled */
> nmax = so->so_rcv.sb_wat;
> else {
>

[PATCH] dont increase the size of socket buffers in low memory situations

[Simon Mages]

At the moment the buffersize will be set to the default even if the
current value
is smaller.

The following diff fixes this problem.

Index: netinet/tcp_usrreq.c
===
RCS file: /cvs/src/sys/netinet/tcp_usrreq.c,v
retrieving revision 1.131
diff -u -p -u -p -r1.131 tcp_usrreq.c
--- netinet/tcp_usrreq.c18 Jun 2016 10:36:13 -  1.131
+++ netinet/tcp_usrreq.c5 Jul 2016 09:26:24 -
@@ -979,10 +979,11 @@ tcp_update_sndspace(struct tcpcb *tp)
struct socket *so = tp->t_inpcb->inp_socket;
u_long nmax;

-   if (sbchecklowmem())
+   if (sbchecklowmem()) {
/* low on memory try to get rid of some */
-   nmax = tcp_sendspace;
-   else if (so->so_snd.sb_wat != tcp_sendspace)
+   if (so->so_snd.sb_hiwat < nmax)
+   nmax = tcp_sendspace;
+   } else if (so->so_snd.sb_wat != tcp_sendspace)
/* user requested buffer size, auto-scaling disabled */
nmax = so->so_snd.sb_wat;
else
@@ -1017,10 +1018,11 @@ tcp_update_rcvspace(struct tcpcb *tp)
struct socket *so = tp->t_inpcb->inp_socket;
u_long nmax = so->so_rcv.sb_hiwat;

-   if (sbchecklowmem())
+   if (sbchecklowmem()) {
/* low on memory try to get rid of some */
-   nmax = tcp_recvspace;
-   else if (so->so_rcv.sb_wat != tcp_recvspace)
+   if (tcp_recvspace < nmax)
+   nmax = tcp_recvspace;
+   } else if (so->so_rcv.sb_wat != tcp_recvspace)
/* user requested buffer size, auto-scaling disabled */
nmax = so->so_rcv.sb_wat;
else {

socket buffers

[Stuart Henderson]

Does anyone know offhand the reason why network connections fail
if socket buffers are set above 256k?

# sysctl net.inet.tcp.sendspace=262145 
# telnet naiad 80
Trying 2a01:348:108:108:a00:20ff:feda:88b6...
Trying 195.95.187.35...
#

I was thinking of looking into it, but before going down that rabbit
hole I thought I'd ask in case there's a quick answer that somebody
already knows...

(yes, people do use buffers much bigger than this, I looked at some
of the academic ftp mirror sites - looks like mirrorservice.org wil
negotiate 3MB buffers, aarnet 35MB, if you let them - presumably
they try to avoid buffers being a bottleneck for clients reaching
them over a national network of at least 1Gb/s end-to-end).

Re: socket buffers

[Joerg Sonnenberger]

On Sat, Jul 03, 2010 at 11:54:17AM +0100, Stuart Henderson wrote:
 Does anyone know offhand the reason why network connections fail
 if socket buffers are set above 256k?

You might have to patch sb_max for that.

Joerg

Re: socket buffers

[Claudio Jeker]

On Sat, Jul 03, 2010 at 11:54:17AM +0100, Stuart Henderson wrote:
 Does anyone know offhand the reason why network connections fail
 if socket buffers are set above 256k?
 

There is this magical define in uipc_socket2.c called SB_MAX that limits
the socket buffers to 256k going over that line makes the initial scaling
fail and you end up with no buffer at all.

 # sysctl net.inet.tcp.sendspace=262145 
 # telnet naiad 80
 Trying 2a01:348:108:108:a00:20ff:feda:88b6...
 Trying 195.95.187.35...
 #
 
 I was thinking of looking into it, but before going down that rabbit
 hole I thought I'd ask in case there's a quick answer that somebody
 already knows...
 
 (yes, people do use buffers much bigger than this, I looked at some
 of the academic ftp mirror sites - looks like mirrorservice.org wil
 negotiate 3MB buffers, aarnet 35MB, if you let them - presumably
 they try to avoid buffers being a bottleneck for clients reaching
 them over a national network of at least 1Gb/s end-to-end).
 

35M, that is insane. Either they have machines with infinite memory or you
can kill the boxes easily.

-- 
:wq Claudio

Re: socket buffers

[Joerg Sonnenberger]

On Sat, Jul 03, 2010 at 05:40:45PM +0200, Claudio Jeker wrote:
 35M, that is insane. Either they have machines with infinite memory or you
 can kill the boxes easily.

You don't need 35MB per client connection if interfaces like sendfile(2)
are used. All the kernel has to guarantee in that case is copy-on-write
for the file content as far as it has been send already. Media
distribution server normally don't change files inplace, so the only
backpressure this creates is on the VFS cache. Let's assume the server
is busy due to a new OpenBSD/Linux/Firefox/whatever release. A lot of
clients will try to fetch a small number of distinct files. The memory
the kernel has to commit is limited by the size of that active set, not
by the number of clients.

Joerg

Re: socket buffers

[Stuart Henderson]

On 2010/07/03 18:17, Joerg Sonnenberger wrote:
 On Sat, Jul 03, 2010 at 05:40:45PM +0200, Claudio Jeker wrote:
  35M, that is insane. Either they have machines with infinite memory or you
  can kill the boxes easily.

some would also say that 16K is insane ;-)

 You don't need 35MB per client connection if interfaces like sendfile(2)
 are used. All the kernel has to guarantee in that case is copy-on-write
 for the file content as far as it has been send already. Media
 distribution server normally don't change files inplace, so the only
 backpressure this creates is on the VFS cache. Let's assume the server
 is busy due to a new OpenBSD/Linux/Firefox/whatever release. A lot of
 clients will try to fetch a small number of distinct files. The memory
 the kernel has to commit is limited by the size of that active set, not
 by the number of clients.

there is some pretty serious hardware behind it...
http://mirror.aarnet.edu.au/indexabout.html

Re: socket buffers

[Rod Whitworth]

On Sat, 3 Jul 2010 17:46:22 +0100, Stuart Henderson wrote:

there is some pretty serious hardware behind it...
http://mirror.aarnet.edu.au/indexabout.html

Those guys have some serious uses for that equipment in addition to
being a great source of ftp mirrors.

They are ready (or very close) to handling data from Australia and New
Zealand SKA sites.
(Square Kilometer Array http://www.skatelescope.org/) The data is
measured in Terabytes/day.

BTW their OpenBSD mirror currently has 4.7 pkgs for some archs (I did
not check all but amd64 is there) but not i386. Weird.
*** NOTE *** Please DO NOT CC me. I am subscribed to the list.
Mail to the sender address that does not originate at the list server is 
tarpitted. The reply-to: address is provided for those who feel compelled to 
reply off list. Thankyou.

Rod/
---
This life is not the real thing.
It is not even in Beta.
If it was, then OpenBSD would already have a man page for it.