From: Jeff King <peff@peff.net>
To: Eric Wong <normalperson@yhbt.net>
Cc: Daniel Stenberg <daniel@haxx.se>,
Junio C Hamano <gitster@pobox.com>,
git@vger.kernel.org
Subject: [PATCH] http: use curl's tcp keepalive if available
Date: Mon, 14 Oct 2013 20:06:14 -0400 [thread overview]
Message-ID: <20131015000614.GA10905@sigill.intra.peff.net> (raw)
In-Reply-To: <20131014233839.GA26323@dcvr.yhbt.net>
On Mon, Oct 14, 2013 at 11:38:39PM +0000, Eric Wong wrote:
> I wanted it to work as older curl first (since I noticed this
> on an old server). But your patch on top of mine looks reasonable,
> thanks.
Makes sense. Here it is with a real commit message (on top of the
ew/keepalive topic).
-- >8 --
Subject: http: use curl's tcp keepalive if available
Commit a15d069 taught git to use curl's SOCKOPTFUNCTION hook
to turn on TCP keepalives. However, modern versions of curl
have a TCP_KEEPALIVE option, which can do this for us. As an
added bonus, the curl code knows how to turn on keepalive
for a much wider variety of platforms. The only downside to
using this option is that not everybody has a new enough curl.
Let's split our keepalive options into three conditionals:
1. With curl 7.25.0 and newer, we rely on curl to do it
right.
2. With older curl that still knows SOCKOPTFUNCTION, we
use the code from a15d069.
3. Otherwise, we are out of luck, and the call is a no-op.
Signed-off-by: Jeff King <peff@peff.net>
---
Given the #ifdefs in curl's keepalive code, I suspect we may see build
problems for people in case 2 on some systems, with or without my patch.
I think this patch is a strict improvement, though; if they have a new
enough curl, they will not even look at the case 2 code. And if they do
not, our previous options were:
a. Add platform-specific code for them.
b. Tell them they are out of luck, and add an #ifdef to push them into
case 3.
Now we have an extra option:
c. Tell them to upgrade curl. :)
http.c | 24 ++++++++++++++++++++----
1 file changed, 20 insertions(+), 4 deletions(-)
diff --git a/http.c b/http.c
index a2c1819..6359526 100644
--- a/http.c
+++ b/http.c
@@ -233,7 +233,13 @@ static int has_cert_password(void)
return 0;
}
-/* curl 7.25.0 has CURLOPT_TCP_KEEPALIVE, too, but we support older curl */
+#if LIBCURL_VERSION_NUM >= 0x071900
+static void set_curl_keepalive(CURL *c)
+{
+ curl_easy_setopt(c, CURLOPT_TCP_KEEPALIVE, 1);
+}
+
+#elif LIBCURL_VERSION_NUM >= 0x071000
static int sockopt_callback(void *client, curl_socket_t fd, curlsocktype type)
{
int ka = 1;
@@ -251,6 +257,18 @@ static int sockopt_callback(void *client, curl_socket_t fd, curlsocktype type)
return 0; /* CURL_SOCKOPT_OK only exists since curl 7.21.5 */
}
+static void set_curl_keepalive(CURL *c)
+{
+ curl_easy_setopt(c, CURLOPT_SOCKOPTFUNCTION, sockopt_callback);
+}
+
+#else
+static void set_curl_keepalive(CURL *c)
+{
+ /* not supported on older curl versions */
+}
+#endif
+
static CURL *get_curl_handle(void)
{
CURL *result = curl_easy_init();
@@ -316,9 +334,7 @@ static CURL *get_curl_handle(void)
if (curl_http_proxy)
curl_easy_setopt(result, CURLOPT_PROXY, curl_http_proxy);
-#if LIBCURL_VERSION_NUM >= 0x071000
- curl_easy_setopt(result, CURLOPT_SOCKOPTFUNCTION, sockopt_callback);
-#endif
+ set_curl_keepalive(result);
return result;
}
--
1.8.4.1.4.gf327177
next prev parent reply other threads:[~2013-10-15 0:06 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-10-12 22:29 [PATCH] http: enable keepalive on TCP sockets Eric Wong
2013-10-13 9:44 ` Daniel Stenberg
2013-10-14 5:27 ` Eric Wong
2013-10-14 21:40 ` Jeff King
2013-10-14 23:38 ` Eric Wong
2013-10-15 0:06 ` Jeff King [this message]
2013-10-15 4:58 ` [PATCH] http: use curl's tcp keepalive if available Eric Wong
2013-10-15 5:00 ` Jeff King
2013-10-15 6:03 ` Eric Wong
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20131015000614.GA10905@sigill.intra.peff.net \
--to=peff@peff.net \
--cc=daniel@haxx.se \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=normalperson@yhbt.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).