Le Thursday 06 December 2007 13:05:09 Daniel Stenberg, vous avez écrit :
> On Wed, 5 Dec 2007, BLANC Gilles wrote:
>
> (Cc'ing this reply over to the curl-users list since now we're talking
> changes to the tool, not the lib, let's continue this thread there!)
done, I've just subscribed.

> > I have indeed added a call to "my_setopt" with the
> > CURLOPT_SOCKOPTFUNCTION tag

> > Here is the patch, only applied to src/main.c (and no longer the lib, so
> > that this message is no longer in the right place I think...). Hope it
> > will be integrated in the mainstream line :) (or at least very useful
> > for weird firewalled guys).

> [patch is here:
> http://curl.haxx.se/mail/lib-2007-12/att-0036/curl-7.17.1-keepalive.patch]

> Thanks!
You're welcome

> A few remarks about the patch:
>
> #1 - is keep-alive really *always* a good idea for the tool or shouldn't it
> be optional with a command line option?
Well, it doesn't hurt at least. Actually, it sends a packet called "keepalive
probe" (with 0 or 1 data octet depending to implementation) to tell the
remote server that it is still alive. If the server has decided to detect
with this standardized method that the peer is up (instead of waiting a very
long time to detect if the peer has died, which is the case almost all the
time), when a socket is opened in stream mode, and whenever the data exchange
is longer than 2 hours, the client will be disconnected (and it's legal !
^^). And one should have to wait two hours before knowing that the remote
system is a little psycho (in our case, it is worse than that, so we also
have to modify the Linux default time between sendings,
in /proc/sys/net/ipv4/tcp_keepalive_time : default is effectively 7200
seconds ; in 2001, it seemed to be 10800, ie 3 hours, so it has been
corrected since then).

Everything is in RFC1122 (TCP), section 4.2.3.6 "TCP Keep-Alives". They tell
us to keep it optional, because it should cost to much bandwith to send a 1
octet packet every two hours (1989....) ; but it talks about the kernel
implementation (and it's indeed optional, called with setsockopt, as we have
seen), not the applications that do data transfer for more than two hours in
stream mode like FTP.

So, we could make it optional (at compilation time or with command line
argument), but as we have seen, it is totally legal to be kicked by a server
after two hours if you don't use it (well, it is not polite, because it
should be it that ask if we are alive, not the client to tell, actually), and
it's only one poor packet lost in gigs of data (because we are not in 1989
anymore, data bandwith are huge by now :) ). You tell me ;).

> #2 - the funny ascii graphics in the outputs don't belong there as nothing
> else in curl do that. It seems using warnf() is the better approach
> for warnings-style output in the tool.
no problem : "warnf(clientp, ..."

> #3 - do all systems have SO_KEEPALIVE? I couldn't find any that don't given
> a quick google.
Neither do I (with a long google), it is POSIX, and even windows have it ;).
But former versions seemed to have problem as client, in answering an ACK to
a probe (which is not our case, because it's us that send a probe for an
hypothetical strict server).
However, m$ also say that "Keep-alive packets are sent by the underlying
network transport. Not all network transports support sending keep-alive
packets." ( http://msdn2.microsoft.com/en-us/library/ms803566.aspx ), with no
more reference, a little sibylic. Because they talk about the TCP support and
not the option itself, so they suppose that there are networks with
incomplete stacks (theirs before ? :s ).

On an old UNIX socket in C FAQ, we can find :
http://www.blacksheepnetworks.com/security/info/unix/sock/unix-socket-faq-2.html#peer_death
which explains exactly what the problem is, and they also recommend the
SO_KEEPALIVE option without any restriction or portability doubt emitted.

Nothing better than that.

> #4 - A minor nuisance is that setsockopt() on POSIX takes a socklen_t type
> in its last argument while for example winsock takes (and I believe some
> pre-POSIX unixes do too) an 'int', so using sizeof() like that will cause a
> warning on some compilers...
Well in the rest of the code, it alters between sizeof (var) and sizeof
(int) ; once, there is a "sizeof (var) < 0", nasty hacks which is not so
stupid after all ^^. I don't have a window compiler near me (I'm an healthy
guy), what would you advise ?

> #5 - your not using 'clientp' will cause compiler warnings, but I figure
> that it'll become used if you fix #2 since that needs a config
> pointer...
done :)

> #6 - the function set_so_keepalive() should be made static
right.

I wait for your precisions/decisions ("option or not, that is the question"),
and the I will post a new patch jus after, if you don't mind :).

Thanks for your time,

-- 
Gilles Blanc - gblanc_at_linagora.com
Ingénieur informatique embarquée
Groupe LINAGORA - http://www.linagora.com
Tél.: +33(0)1 58 18 68 28 - Fax : +33(0)1 58 18 68 29