Mailing List Archive: votes for current patch round...

votes for current patch round...

Sep 5, 1995, 10:17 AM

Post #1 of 9 (483 views)

First, explanations for the vetoes:

interrupt_accept:

I've already aired my objections to this one, but for the record:

By all accounts, the pause problems on linux, as with similar
problems long since observed on other systems, are due to the number
of pending connections on the port 80 socket exceeding the kernel's
fixed limit. This patch does not cure that problem. Furthermore,
if the problem is cured properly (by raising the limit, however you
do that on Linux --- it wouldn't surprise me if it requires
recompiling the kernel, since that's what's needed on SunOS), then
the patch is not needed.

What's more, this particular patch actually introduces a race
condition with similar symptoms --- if the alarm() triggers after
accept() returns but before the alarm() itself can be disarmed, then
the connection will not be serviced, and the descriptor will leak.
The way this looks on the client side is that the browser hangs
forever waiting for a response (nearly indistinguishable from the
case where it hangs waiting for the connection to complete).

If there is a separate Linux bug, different from the
pending-connections limit, which is cured by this patch, I'll lift
the veto, but that can only be tested by running an unmodified
server with a fixed kernel.

utsname:

gethostname() works. No reason not to use it.

itime:

Does not compile on SunOS, apparently due to a missing header file.
I don't have any fundamental problems with this patch, and would be
happy to vote +1 if it even compiled cleanly, but it doesn't....

The complete votes:

+1 01_lock_fname.0.8.12.patch
+1 02_NeXT_fixes.0.8.12.patch
-1 03_utsname_svr4.0.8.12.patch
+0 04_util_cleanup.0.8.12.patch --- (char*) would be better than
(void*); it matches the prototype.
+1 05_nego_cleanup.0.8.12.patch
+1 06_ignore_index.0.8.12.patch
-1 07_interrupt_accept.0.8.12.patch
+0 08b_LynxOS_new_gmtoff.0.8.12.patch
+0 08c_LynxOS_new_gmtoff.0.8.12.patch --- NB this only works on LynxOS
if the LynxOS linker can cope with get_gmtoff being compiled into
*every* file, including util.c, which has another definition.
Can't say that it doesn't, since I don't run on LynxOS, but it
certainly looks weird. NB this also conflicts with no_bsd_conf.
+1 09_no_bsd_conf.0.8.12.patch
+1 10_dynamic_load.0.8.12.patch
+1 11_imap_point.0.8.12.patch
+1 12_virt_server_name.0.8.12.patch
+0 13_new_icons
+1 14_mod_dir_cosmetics.0.8.12.patch
-1 15_log_config_integer_time.0.8.12.patch
+1 16_virtual_maxopenfiles.0.8.12.patch

Re: votes for current patch round... [ In reply to ]

awm at qosina

Sep 5, 1995, 8:24 AM

Post #2 of 9 (477 views)

Permalink

At 10:17 AM 9/5/95 EDT, you wrote:
>First, explanations for the vetoes:
>
>interrupt_accept:
>
> I've already aired my objections to this one, but for the record:
>
> By all accounts, the pause problems on linux, as with similar
> problems long since observed on other systems, are due to the number
> of pending connections on the port 80 socket exceeding the kernel's
> fixed limit. This patch does not cure that problem. Furthermore,
> if the problem is cured properly (by raising the limit, however you
> do that on Linux --- it wouldn't surprise me if it requires
> recompiling the kernel, since that's what's needed on SunOS), then
> the patch is not needed.

This is not a kernel bug. It's a TCP bug, which recompiling the kernel
does not fix. There is no value in linux for this to increase. Linux's kernel
is all source, so it's not as complicated as SunOS and such... anyway as
I said, we really don't have to include the patch in the disterbution, but have
it as a 'if you're having problems try this patch'. I belive it should be
included
as a http_main.c.linux file.

Another pointer I would like to make is, whatever problems it may cause on
other system, the combination of this patch, and linux-1.2.13 does fix the
problem, maybe it should be a #ifdef LINUX <patch> #else <nopatch> #endif
case?

>
> If there is a separate Linux bug, different from the
> pending-connections limit, which is cured by this patch, I'll lift
> the veto, but that can only be tested by running an unmodified
> server with a fixed kernel.

That's all I belive... no other problems.

--
Aram W. Mirzadeh, MIS Manager, Qosina Corporation
http://www.qosina.com/~awm/, awm@qosina.com
Apache httpd server team http://www.apache.org

Re: votes for current patch round... [ In reply to ]

chuck at telebase

Sep 5, 1995, 10:14 AM

Post #3 of 9 (481 views)

Permalink

Re: votes for current patch round... [ In reply to ]

awm at qosina

Sep 5, 1995, 11:28 AM

Post #4 of 9 (481 views)

Permalink

If you check Linux 1.2.13 they have remade this section. I've talked with
the Linux networking people and do not recommend playing with this
part of the code.

1.3.x is supposedly rewritting this entire thing. If you like to follow the
discussion you can send a message to Majordomo@vger.rutgers.edu
with subscribe linux-kernel <address> in '<>'. They're talking about it
right now.

<Aram>

At 01:14 PM 9/5/95 -0400, you wrote:
>Please, this is a Linux kernel patch. The relevant function from Linux'
>net/af_inet.c is shown below, with its hardcoded listen backlog limit.
>
>This is from Linux 1.2.9.
>
>static int inet_listen(struct socket *sock, int backlog)
>{
> struct sock *sk = (struct sock *) sock->data;
>
> if(inet_autobind(sk)!=0)
> return -EAGAIN;
>
> /* We might as well re use these. */
> /*
> * note that the backlog is "unsigned char", so truncate it
> * somewhere. We might as well truncate it to what everybody
> * else does..
> */
> if (backlog > 5)
> backlog = 5;
> sk->max_ack_backlog = backlog;
> if (sk->state != TCP_LISTEN)
> {
> sk->ack_backlog = 0;
> sk->state = TCP_LISTEN;
> }
> return(0);
>}
>
>
>aram W. Mirzadeh liltingly intones:
>>
>> At 10:17 AM 9/5/95 EDT, you wrote:
>> >First, explanations for the vetoes:
>> >
>> >interrupt_accept:
>> >
>> > I've already aired my objections to this one, but for the record:
>> >
>> > By all accounts, the pause problems on linux, as with similar
>> > problems long since observed on other systems, are due to the number
>> > of pending connections on the port 80 socket exceeding the kernel's
>> > fixed limit. This patch does not cure that problem. Furthermore,
>> > if the problem is cured properly (by raising the limit, however you
>> > do that on Linux --- it wouldn't surprise me if it requires
>> > recompiling the kernel, since that's what's needed on SunOS), then
>> > the patch is not needed.
>>
>> This is not a kernel bug. It's a TCP bug, which recompiling the kernel
>> does not fix. There is no value in linux for this to increase. Linux's
kernel
>> is all source, so it's not as complicated as SunOS and such... anyway as
>> I said, we really don't have to include the patch in the disterbution,
but have
>> it as a 'if you're having problems try this patch'. I belive it should be
>> included
>> as a http_main.c.linux file.
>>
>> Another pointer I would like to make is, whatever problems it may cause on
>> other system, the combination of this patch, and linux-1.2.13 does fix the
>> problem, maybe it should be a #ifdef LINUX <patch> #else <nopatch> #endif
>> case?
>>
>> >
>> > If there is a separate Linux bug, different from the
>> > pending-connections limit, which is cured by this patch, I'll lift
>> > the veto, but that can only be tested by running an unmodified
>> > server with a fixed kernel.
>>
>> That's all I belive... no other problems.
>>
>> --
>> Aram W. Mirzadeh, MIS Manager, Qosina Corporation
>> http://www.qosina.com/~awm/, awm@qosina.com
>> Apache httpd server team http://www.apache.org
>>
>>
>>
>
>chuck
>Chuck Murcko Telebase Systems, Inc. Wayne PA chuck@telebase.com
>And now, on a lighter note:
>Radioactive cats have 18 half-lives.
>
>
--
Aram W. Mirzadeh, MIS Manager, Qosina Corporation
http://www.qosina.com/~awm/, awm@qosina.com
Apache httpd server team http://www.apache.org

Re: votes for current patch round... [ In reply to ]

awm at qosina

Sep 5, 1995, 3:05 PM

Post #5 of 9 (481 views)

Permalink

At 05:18 PM 9/5/95 EDT, you wrote:
>Aram, Chuck's point (and mine) is that this is a problem, not with TCP/IP
>as a whole, but with the Linux TCP/IP implementation (as your Linux gurus
>seem to confirm by saying that they're working on it). It cannot be fixed
>or worked around by changing the server, or any other user-mode code; when
>the kernel is fixed, the problem will vanish.

I totally agree.

>
>In the meantime, raising the fixed limit of 5 in the function which Chuck
>quoted doesn't seem like anything subtle or difficult to me... it's basically
>what one does on SunOS, although on this system, the constant in question is
>at least in a header file, and not buried in the source...

You see the type of questions we get around here that are from Linux people....
I would actually say rebuilding a kernel in SunOS is an easy matter....
Linux on
the other hand... is another question. Most people (60%) who run linux are
just
getting into unix. Trying to get them to DO a kernel rebuild is hard enough,
not even mentioning trying to patch it.

All I need is to tell people when they say, "hey I'm having this problem, what's
the deal?" I rather say, 'we know the problem, here is a temporary fix.' Or
'It's not our problem, it's a linux bug.'

<Aram>
--
Aram W. Mirzadeh, MIS Manager, Qosina Corporation
http://www.qosina.com/~awm/, awm@qosina.com
Apache httpd server team http://www.apache.org

Re: votes for current patch round... [ In reply to ]

rst at ai

Sep 5, 1995, 5:18 PM

Post #6 of 9 (480 views)

Permalink

Aram, Chuck's point (and mine) is that this is a problem, not with TCP/IP
as a whole, but with the Linux TCP/IP implementation (as your Linux gurus
seem to confirm by saying that they're working on it). It cannot be fixed
or worked around by changing the server, or any other user-mode code; when
the kernel is fixed, the problem will vanish.

In the meantime, raising the fixed limit of 5 in the function which Chuck
quoted doesn't seem like anything subtle or difficult to me... it's basically
what one does on SunOS, although on this system, the constant in question is
at least in a header file, and not buried in the source...

rst

Re: votes for current patch round... [ In reply to ]

rst at ai

Sep 5, 1995, 8:22 PM

Post #7 of 9 (479 views)

Permalink

The trouble is that while we know what the problem is, we *can't* produce
a "temporary fix" which will cure it, because the bug is in the kernel,
where our code can't work around it.

You may not like to say "It's not our problem, it's a linux bug", but that
is in fact the situation, and fixing linux is the only way to handle it.

(Recompiling the kernel is a gurus-only procedure even on SunOS, by the
way, but if you want to run a busy server, you're stuck with it... hopefully,
if there's another Linux 1.2.x release, this limit will get raised to a more
reasonable value, something closer to the apparent hard maximum of 255
which comes from the maximum value of an unsigned char...).

rst

Re: votes for current patch round... [ In reply to ]

cloos at JHCloos

Sep 6, 1995, 1:21 AM

Post #8 of 9 (479 views)

Permalink

Robert> In the meantime, raising the fixed limit of 5 in the function
Robert> which Chuck quoted doesn't seem like anything subtle or
Robert> difficult to me... it's basically what one does on SunOS,
Robert> although on this system, the constant in question is at least
Robert> in a header file, and not buried in the source...

I'd note that in 1.3.15, the default is 128, not 5. This is still not
enough here (between 125K and 250K hits/day).

It could be made as high as 255, w/o redoing the struct sock.

-JimC
--
James H. Cloos, Jr. Snail: POBox 18122 Austin, TX 78760-8122 Usenix,SAGE
cloos@io.com URL: http://www.jhcloos.com/~cloos/ LPF