Discussion:
[PATCH] curl: Support CURLOPT_LOCALADDR to bind to local address.
g***@candelatech.com
2017-11-09 15:17:20 UTC
Permalink
From: Ben Greear <***@candelatech.com>

This allows a user to bind to both an interface (with CURLOPT_INTERFACE)
and a local IP address. In doing so, it allows the user to work-around
some serious performance issues on machines with lots of virtual interfaces
because curl no longer has to scan all interfaces each time it makes
a connection.

Signed-off-by: Ben Greear <***@candelatech.com>
---

We have been using this for some time in one form or another. It
is messing with some tricky code though, so it could use a good
review.

docs/libcurl/curl_easy_setopt.3 | 2 +
docs/libcurl/opts/CURLOPT_LOCALADDR.3 | 46 ++++++++++
include/curl/curl.h | 3 +
include/curl/typecheck-gcc.h | 1 +
lib/connect.c | 158 +++++++++++++++++++++-------------
lib/url.c | 23 ++++-
lib/urldata.h | 2 +
packages/OS400/ccsidcurl.c | 1 +
src/tool_cfgable.c | 1 +
src/tool_cfgable.h | 1 +
src/tool_getparam.c | 5 ++
src/tool_help.c | 2 +
src/tool_operate.c | 2 +
13 files changed, 185 insertions(+), 62 deletions(-)
create mode 100644 docs/libcurl/opts/CURLOPT_LOCALADDR.3

diff --git a/docs/libcurl/curl_easy_setopt.3 b/docs/libcurl/curl_easy_setopt.3
index 2982056..ea79af8 100644
--- a/docs/libcurl/curl_easy_setopt.3
+++ b/docs/libcurl/curl_easy_setopt.3
@@ -185,6 +185,8 @@ Proxy authentication service name. \fICURLOPT_PROXY_SERVICE_NAME(3)\fP
Authentication service name. \fICURLOPT_SERVICE_NAME(3)\fP
.IP CURLOPT_INTERFACE
Bind connection locally to this. See \fICURLOPT_INTERFACE(3)\fP
+.IP CURLOPT_LOCALADDR
+Set local IP address to use. See \fICURLOPT_LOCALADDR(3)\fP
.IP CURLOPT_LOCALPORT
Bind connection locally to this port. See \fICURLOPT_LOCALPORT(3)\fP
.IP CURLOPT_LOCALPORTRANGE
diff --git a/docs/libcurl/opts/CURLOPT_LOCALADDR.3 b/docs/libcurl/opts/CURLOPT_LOCALADDR.3
new file mode 100644
index 0000000..fc17ec5
--- /dev/null
+++ b/docs/libcurl/opts/CURLOPT_LOCALADDR.3
@@ -0,0 +1,46 @@
+.\" **************************************************************************
+.\" * _ _ ____ _
+.\" * Project ___| | | | _ \| |
+.\" * / __| | | | |_) | |
+.\" * | (__| |_| | _ <| |___
+.\" * \___|\___/|_| \_\_____|
+.\" *
+.\" * Copyright (C) 1998 - 2014, Daniel Stenberg, <***@haxx.se>, et al.
+.\" *
+.\" * This software is licensed as described in the file COPYING, which
+.\" * you should have received as part of this distribution. The terms
+.\" * are also available at http://curl.haxx.se/docs/copyright.html.
+.\" *
+.\" * You may opt to use, copy, modify, merge, publish, distribute and/or sell
+.\" * copies of the Software, and permit persons to whom the Software is
+.\" * furnished to do so, under the terms of the COPYING file.
+.\" *
+.\" * This software is distributed on an "AS IS" basis, WITHOUT WARRANTY OF ANY
+.\" * KIND, either express or implied.
+.\" *
+.\" **************************************************************************
+.\"
+.TH CURLOPT_LOCALADDR 3 "24 Feb 2015" "libcurl 7.37.0" "curl_easy_setopt options"
+.SH NAME
+CURLOPT_LOCALADDR \- source IP address for outgoing traffic
+.SH SYNOPSIS
+#include <curl/curl.h>
+
+CURLcode curl_easy_setopt(CURL *handle, CURLOPT_LOCALADDR, char *ip_addr);
+.SH DESCRIPTION
+Pass a char * as parameter. This sets the \fIP Address\fP to use as
+for outgoing connections.
+
+.SH DEFAULT
+NULL, use whatever the TCP stack finds suitable
+.SH PROTOCOLS
+All
+.SH EXAMPLE
+TODO
+.SH AVAILABILITY
+Not upstream yet.
+.SH RETURN VALUE
+Returns CURLE_OK on success or
+CURLE_OUT_OF_MEMORY if there was insufficient heap space.
+.SH "SEE ALSO"
+.BR CURLOPT_SOCKOPTFUNCTION "(3), " CURLOPT_INTERFACE "(3), "
diff --git a/include/curl/curl.h b/include/curl/curl.h
index 6803a2e..b9e155c 100644
--- a/include/curl/curl.h
+++ b/include/curl/curl.h
@@ -1815,6 +1815,9 @@ typedef enum {
/* Post MIME data. */
CINIT(MIMEPOST, OBJECTPOINT, 269),

+ /* Set the IP-Address string to use as outgoing IP Addr */
+ CINIT(LOCALADDR, OBJECTPOINT, 270),
+
CURLOPT_LASTENTRY /* the last unused */
} CURLoption;

diff --git a/include/curl/typecheck-gcc.h b/include/curl/typecheck-gcc.h
index 10c74c7..b825892 100644
--- a/include/curl/typecheck-gcc.h
+++ b/include/curl/typecheck-gcc.h
@@ -270,6 +270,7 @@ _CURL_WARNING(_curl_easy_getinfo_err_curl_off_t,
(option) == CURLOPT_ISSUERCERT || \
(option) == CURLOPT_KEYPASSWD || \
(option) == CURLOPT_KRBLEVEL || \
+ (option) == CURLOPT_LOCALADDR || \
(option) == CURLOPT_LOGIN_OPTIONS || \
(option) == CURLOPT_MAIL_AUTH || \
(option) == CURLOPT_MAIL_FROM || \
diff --git a/lib/connect.c b/lib/connect.c
index 927ff7c..54c5141 100755
--- a/lib/connect.c
+++ b/lib/connect.c
@@ -255,7 +255,30 @@ static CURLcode bindlocal(struct connectdata *conn,
/* how many port numbers to try to bind to, increasing one at a time */
int portnum = data->set.localportrange;
const char *dev = data->set.str[STRING_DEVICE];
+ const char *addr = data->set.str[STRING_LOCALADDR];
int error;
+ bool is_interface = FALSE;
+ bool is_host = FALSE;
+ static const char *if_prefix = "if!";
+ static const char *host_prefix = "host!";
+ bool resolv_myhost = false;
+ char myhost[256] = "";
+ int done = 0;
+
+ memset(&sa, 0, sizeof(struct Curl_sockaddr_storage));
+
+ infof(data, "bind-local, addr: %s dev: %s\n",
+ addr, dev);
+
+ /* The original code only took 'dev', which could be device name or addr.
+ * If only addr is set, then just pretend it was 'dev' to re-use as much
+ * of the old logic as possible. */
+ if(addr && !dev) {
+ is_host = TRUE;
+ dev = addr;
+ addr = NULL;
+ goto decided_is_host;
+ }

/*************************************************************
* Select device to bind socket to
@@ -264,16 +287,7 @@ static CURLcode bindlocal(struct connectdata *conn,
/* no local kind of binding was requested */
return CURLE_OK;

- memset(&sa, 0, sizeof(struct Curl_sockaddr_storage));
-
if(dev && (strlen(dev)<255) ) {
- char myhost[256] = "";
- int done = 0; /* -1 for error, 1 for address found */
- bool is_interface = FALSE;
- bool is_host = FALSE;
- static const char *if_prefix = "if!";
- static const char *host_prefix = "host!";
-
if(strncmp(if_prefix, dev, strlen(if_prefix)) == 0) {
dev += strlen(if_prefix);
is_interface = TRUE;
@@ -283,59 +297,78 @@ static CURLcode bindlocal(struct connectdata *conn,
is_host = TRUE;
}

+ decided_is_host:
/* interface */
- if(!is_host) {
- switch(Curl_if2ip(af, scope, conn->scope_id, dev,
- myhost, sizeof(myhost))) {
- case IF2IP_NOT_FOUND:
- if(is_interface) {
- /* Do not fall back to treating it as a host name */
- failf(data, "Couldn't bind to interface '%s'", dev);
- return CURLE_INTERFACE_FAILED;
- }
- break;
- case IF2IP_AF_NOT_SUPPORTED:
- /* Signal the caller to try another address family if available */
- return CURLE_UNSUPPORTED_PROTOCOL;
- case IF2IP_FOUND:
- is_interface = TRUE;
- /*
- * We now have the numerical IP address in the 'myhost' buffer
- */
- infof(data, "Local Interface %s is ip %s using address family %i\n",
- dev, myhost, af);
- done = 1;
+
+ if((!is_host) && (is_interface || Curl_if_is_interface_name(dev))) {
+ is_interface = TRUE; /* in case we had to call Curl_if_is_interface...*/
+ if(addr && dev) {
+ strncpy(myhost, addr, sizeof(myhost));
+ myhost[sizeof(myhost)-1] = 0;
+ resolv_myhost = TRUE;
+ }
+ else {
+ /* Discover IP addr for interface */
+ switch(Curl_if2ip(af, scope, conn->scope_id, dev,
+ myhost, sizeof(myhost))) {
+ case IF2IP_NOT_FOUND:
+ if(is_interface) {
+ /* Do not fall back to treating it as a host name */
+ failf(data, "Couldn't bind to interface '%s', addr: '%s'",
+ dev, addr);
+ return CURLE_INTERFACE_FAILED;
+ }
+ break;
+ case IF2IP_AF_NOT_SUPPORTED:
+ /* Signal the caller to try another address family if available */
+ return CURLE_UNSUPPORTED_PROTOCOL;
+ case IF2IP_FOUND:
+ is_interface = TRUE;
+ /*
+ * We now have the numerical IP address in the 'myhost' buffer
+ */
+ infof(data,
+ "Local Interface %s is ip %s using address family %i\n",
+ dev, myhost, af);
+ done = 1;
+ break;
+ }/* switch */
+ }/* else */

#ifdef SO_BINDTODEVICE
- /* I am not sure any other OSs than Linux that provide this feature,
- * and at the least I cannot test. --Ben
- *
- * This feature allows one to tightly bind the local socket to a
- * particular interface. This will force even requests to other
- * local interfaces to go out the external interface.
- *
- *
- * Only bind to the interface when specified as interface, not just
- * as a hostname or ip address.
- */
- if(setsockopt(sockfd, SOL_SOCKET, SO_BINDTODEVICE,
- dev, (curl_socklen_t)strlen(dev)+1) != 0) {
- error = SOCKERRNO;
- infof(data, "SO_BINDTODEVICE %s failed with errno %d: %s;"
- " will do regular bind\n",
- dev, error, Curl_strerror(conn, error));
- /* This is typically "errno 1, error: Operation not permitted" if
- you're not running as root or another suitable privileged
- user */
- }
-#endif
- break;
+ /* I am not sure any other OSs than Linux that provide this feature,
+ * and at the least I cannot test. --Ben
+ *
+ * This feature allows one to tightly bind the local socket to a
+ * particular interface. This will force even requests to other
+ * local interfaces to go out the external interface.
+ *
+ *
+ * Only bind to the interface when specified as interface, not just
+ * as a hostname or ip address.
+ */
+ if(setsockopt(sockfd, SOL_SOCKET, SO_BINDTODEVICE,
+ dev, (curl_socklen_t)strlen(dev)+1) != 0) {
+ error = SOCKERRNO;
+ infof(data, "SO_BINDTODEVICE %s failed with errno %d: %s;"
+ " will do regular bind\n",
+ dev, error, Curl_strerror(conn, error));
+ /* This is typically "errno 1, error: Operation not permitted" if
+ you're not running as root or another suitable privileged
+ user */
}
+#endif
+ }
+ else {
+ strncpy(myhost, dev, sizeof(myhost));
+ myhost[sizeof(myhost)-1] = 0;
+ resolv_myhost = TRUE;
}
- if(!is_interface) {
+
+ if(resolv_myhost || !is_interface) {
/*
- * This was not an interface, resolve the name as a host name
- * or IP number
+ * addr was specified, or this was not an interface.
+ * Resolve the name as a host name or IP number
*
* Temporarily force name resolution to use only the address type
* of the connection. The resolve functions should really be changed
@@ -351,7 +384,7 @@ static CURLcode bindlocal(struct connectdata *conn,
conn->ip_version = CURL_IPRESOLVE_V6;
#endif

- rc = Curl_resolv(conn, dev, 0, &h);
+ rc = Curl_resolv(conn, myhost, 0, &h);
if(rc == CURLRESOLV_PENDING)
(void)Curl_resolver_wait_resolv(conn, &h);
conn->ip_version = ipver;
@@ -408,7 +441,9 @@ static CURLcode bindlocal(struct connectdata *conn,
}

if(done < 1) {
- failf(data, "Couldn't bind to '%s'", dev);
+ failf(data, "Couldn't bind to '%s', addr '%s', done: %d"
+ " is-host: %d is-interface: %d",
+ dev, addr, done, is_host, is_interface);
return CURLE_INTERFACE_FAILED;
}
}
@@ -462,8 +497,11 @@ static CURLcode bindlocal(struct connectdata *conn,
}

data->state.os_errno = error = SOCKERRNO;
- failf(data, "bind failed with errno %d: %s",
- error, Curl_strerror(conn, error));
+ failf(data, "bindlocal: bind failed with errno %d: %s, dev: %s "
+ "is_host: %i is_interface: %i port: %hu addr: %s "
+ "sock->sa_family: %hu myhost: %s resolv-myhost: %i af: %i",
+ error, Curl_strerror(conn, error), dev, is_host, is_interface,
+ port, addr, sock->sa_family, myhost, resolv_myhost, af);

return CURLE_INTERFACE_FAILED;
}
diff --git a/lib/url.c b/lib/url.c
index 9799096..ef2e6f2 100644
--- a/lib/url.c
+++ b/lib/url.c
@@ -2102,6 +2102,14 @@ CURLcode Curl_setopt(struct Curl_easy *data, CURLoption option,
result = setstropt(&data->set.str[STRING_DEVICE],
va_arg(param, char *));
break;
+ case CURLOPT_LOCALADDR:
+ /*
+ * Set what local address to bind the socket to when
+ * performing an operation and thus what from-IP your connection will use.
+ */
+ result = setstropt(&data->set.str[STRING_LOCALADDR],
+ va_arg(param, char *));
+ break;
case CURLOPT_LOCALPORT:
/*
* Set what local port to bind the socket to when performing an operation.
@@ -3072,6 +3080,7 @@ static void conn_free(struct connectdata *conn)
Curl_llist_destroy(&conn->recv_pipe, NULL);

Curl_safefree(conn->localdev);
+ Curl_safefree(conn->localaddr);
Curl_free_primary_ssl_config(&conn->ssl_config);
Curl_free_primary_ssl_config(&conn->proxy_ssl_config);

@@ -3701,7 +3710,7 @@ ConnectionExists(struct Curl_easy *data,
already in use so we skip it */
continue;

- if(needle->localdev || needle->localport) {
+ if(needle->localdev || needle->localport || needle->localaddr) {
/* If we are bound to a specific local end (IP+port), we must not
re-use a random other one, although if we didn't ask for a
particular one we can reuse one that was bound.
@@ -3716,7 +3725,10 @@ ConnectionExists(struct Curl_easy *data,
if((check->localport != needle->localport) ||
(check->localportrange != needle->localportrange) ||
(needle->localdev &&
- (!check->localdev || strcmp(check->localdev, needle->localdev))))
+ (!check->localdev || strcmp(check->localdev, needle->localdev))) ||
+ (needle->localaddr &&
+ (!check->localaddr ||
+ strcmp(check->localaddr, needle->localaddr))))
continue;
}

@@ -4329,6 +4341,11 @@ static struct connectdata *allocate_conn(struct Curl_easy *data)
if(!conn->localdev)
goto error;
}
+ if(data->set.str[STRING_LOCALADDR]) {
+ conn->localaddr = strdup(data->set.str[STRING_LOCALADDR]);
+ if(!conn->localaddr)
+ goto error;
+ }
conn->localportrange = data->set.localportrange;
conn->localport = data->set.localport;

@@ -4346,6 +4363,7 @@ static struct connectdata *allocate_conn(struct Curl_easy *data)
free(conn->master_buffer);
free(conn->localdev);
free(conn);
+ free(conn->localaddr);
return NULL;
}

@@ -6353,6 +6371,7 @@ static void reuse_conn(struct connectdata *old_conn,
Curl_safefree(old_conn->http_proxy.passwd);
Curl_safefree(old_conn->socks_proxy.passwd);
Curl_safefree(old_conn->localdev);
+ Curl_safefree(old_conn->localaddr);

Curl_llist_destroy(&old_conn->send_pipe, NULL);
Curl_llist_destroy(&old_conn->recv_pipe, NULL);
diff --git a/lib/urldata.h b/lib/urldata.h
index 09ac9d1..d8c3d2c 100644
--- a/lib/urldata.h
+++ b/lib/urldata.h
@@ -1020,6 +1020,7 @@ struct connectdata {
that subsequent bound-requested connections aren't accidentally re-using
wrong connections. */
char *localdev;
+ char *localaddr; /* Local addr in dot notation */
unsigned short localport;
int localportrange;
struct http_connect_state *connect_state; /* for HTTP CONNECT */
@@ -1478,6 +1479,7 @@ enum dupstring {
Each such pointer must be added manually to Curl_dupset() --- */

STRING_COPYPOSTFIELDS, /* if POST, set the fields' values here */
+ STRING_LOCALADDR, /* Local IP Addr to use. */

STRING_LAST /* not used, just an end-of-list marker */
};
diff --git a/packages/OS400/ccsidcurl.c b/packages/OS400/ccsidcurl.c
index de2c9cc..3aa33a6 100644
--- a/packages/OS400/ccsidcurl.c
+++ b/packages/OS400/ccsidcurl.c
@@ -1152,6 +1152,7 @@ curl_easy_setopt_ccsid(CURL * curl, CURLoption tag, ...)
case CURLOPT_FTP_ACCOUNT:
case CURLOPT_FTP_ALTERNATIVE_TO_USER:
case CURLOPT_INTERFACE:
+ case CURLOPT_LOCALADDR:
case CURLOPT_ISSUERCERT:
case CURLOPT_KEYPASSWD:
case CURLOPT_KRBLEVEL:
diff --git a/src/tool_cfgable.c b/src/tool_cfgable.c
index 755195c..11a6fda 100644
--- a/src/tool_cfgable.c
+++ b/src/tool_cfgable.c
@@ -61,6 +61,7 @@ static void free_config_fields(struct OperationConfig *config)
Curl_safefree(config->headerfile);
Curl_safefree(config->ftpport);
Curl_safefree(config->iface);
+ Curl_safefree(config->localaddr);

Curl_safefree(config->range);

diff --git a/src/tool_cfgable.h b/src/tool_cfgable.h
index 23943fe..05a63f0 100644
--- a/src/tool_cfgable.h
+++ b/src/tool_cfgable.h
@@ -69,6 +69,7 @@ struct OperationConfig {
char *headerfile;
char *ftpport;
char *iface;
+ char *localaddr; /* local IP address */
int localport;
int localportrange;
unsigned short porttouse;
diff --git a/src/tool_getparam.c b/src/tool_getparam.c
index 31fc7a1..2f7671b 100644
--- a/src/tool_getparam.c
+++ b/src/tool_getparam.c
@@ -108,6 +108,7 @@ static const struct LongShort aliases[]= {
{"*u", "crlf", ARG_BOOL},
{"*v", "stderr", ARG_STRING},
{"*w", "interface", ARG_STRING},
+ {"*W", "localaddr", ARG_STRING},
{"*x", "krb", ARG_STRING},
{"*x", "krb4", ARG_STRING},
/* 'krb4' is the previous name */
@@ -745,6 +746,10 @@ ParameterError getparameter(const char *flag, /* f or -long-flag */
/* interface */
GetStr(&config->iface, nextarg);
break;
+ case 'W': /* --localaddr */
+ /* addr in dot notation */
+ GetStr(&config->localaddr, nextarg);
+ break;
case 'x': /* --krb */
/* kerberos level string */
if(curlinfo->features & CURL_VERSION_KERBEROS4)
diff --git a/src/tool_help.c b/src/tool_help.c
index 486f65d..9af98db 100644
--- a/src/tool_help.c
+++ b/src/tool_help.c
@@ -184,6 +184,8 @@ static const struct helptxt helptext[] = {
"Allow insecure server connections when using SSL"},
{" --interface <name>",
"Use network INTERFACE (or address)"},
+ {" --localaddr <IP-ADDR>",
+ "Specify local IP-Address to use"},
{"-4, --ipv4",
"Resolve names to IPv4 addresses"},
{"-6, --ipv6",
diff --git a/src/tool_operate.c b/src/tool_operate.c
index 25ea9c4..6325492 100644
--- a/src/tool_operate.c
+++ b/src/tool_operate.c
@@ -1237,6 +1237,8 @@ static CURLcode operate_do(struct GlobalConfig *global,
my_setopt_str(curl, CURLOPT_INTERFACE, config->iface);
my_setopt_str(curl, CURLOPT_KRBLEVEL, config->krblevel);

+ my_setopt_str(curl, CURLOPT_LOCALADDR, config->localaddr);
+
progressbarinit(&progressbar, config);
if((global->progressmode == CURL_PROGRESS_BAR) &&
!global->noprogress && !global->mute) {
--
2.4.11

-------------------------------------------------------------------
Unsubscribe: https://cool.haxx.se/list/listinfo/curl-library
Etiquette: https://curl.haxx.se/mail/e
Daniel Stenberg
2017-12-13 08:19:57 UTC
Permalink
On Thu, 9 Nov 2017, ***@candelatech.com wrote:

Hi Ben!
This allows a user to bind to both an interface (with CURLOPT_INTERFACE) and
a local IP address. In doing so, it allows the user to work-around some
serious performance issues on machines with lots of virtual interfaces
because curl no longer has to scan all interfaces each time it makes a
connection.
This patch has clearly been neglected by me, and while I'm not quite ready to
take it on just yet[*] I wanted to ask if you could consider pushing this to
github as a PR?

That way we'd get it tested and verified much better before merge, and it also
helps out to tell us when it starts to get merge confliects (and it makes it
harder for us/me to forget it). Like it does now: the changes in lib/connect.c
needs a rebase/edit to apply on current git master.

Further: I would like to get a clearer view on what exactly CURLOPT_LOCALADDR
provides and offers that CURLOPT_INTERFACE prefixed with "host!" doesn't do?
And if so, can't we instead make CURLOPT_INTERFACE use that? Or perhaps even
add another prefix for CURLOPT_INTERFACE if it truly has to be different. To
me, this feels like adding an entirely new easy option for a very minor
variation of an already existing option.

[*] = I'll take off on an extended vacation now for a few weeks ahead so I
personally won't work on anything larger until I get back.
--
/ daniel.haxx.se
-------------------------------------------------------------------
Unsubscribe: https://cool.haxx.se/list/listinfo/curl-library
Etiquette: https://curl.haxx.se/
Ben Greear
2017-12-13 16:14:43 UTC
Permalink
Post by Daniel Stenberg
Hi Ben!
This allows a user to bind to both an interface (with CURLOPT_INTERFACE) and a local IP address. In doing so, it allows the user to work-around some serious performance issues on machines with lots of virtual interfaces because curl no longer has to scan all interfaces each time it makes a connection.
This patch has clearly been neglected by me, and while I'm not quite ready to take it on just yet[*] I wanted to ask if you could consider pushing this to github as a PR?
I attempted to do this just now...
Post by Daniel Stenberg
That way we'd get it tested and verified much better before merge, and it also helps out to tell us when it starts to get merge confliects (and it makes it harder for us/me to forget it). Like it does now: the changes in lib/connect.c needs a rebase/edit to apply on current git master.
Further: I would like to get a clearer view on what exactly CURLOPT_LOCALADDR provides and offers that CURLOPT_INTERFACE prefixed with "host!" doesn't do? And if so, can't we instead make CURLOPT_INTERFACE use that? Or perhaps even add another prefix for CURLOPT_INTERFACE if it truly has to be different. To me, this feels like adding an entirely new easy option for a very minor variation of an already existing option.
The problem is that you need to be able to specify both an interface and an IP address in some
cases. Think about a system with multiple interfaces, and with at least some interfaces holding
multiple IP addresses.

Also, in cases where you have thousands of interfaces and thousands of IP addresses on a system,
it can be much more efficient to not have to do an exhaustive linear search of the IP addresses
when binding to a particular interface.

Thanks,
Ben
Post by Daniel Stenberg
[*] = I'll take off on an extended vacation now for a few weeks ahead so I personally won't work on anything larger until I get back.
--
Ben Greear <***@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
-------------------------------------------------------------------
Unsubscribe: https://cool.haxx.se/list/listinfo/curl-library
Etiquette: https://curl.haxx.se/mail/etiq
Ben Greear
2018-01-30 21:22:17 UTC
Permalink
Post by Daniel Stenberg
Hi Ben!
This allows a user to bind to both an interface (with CURLOPT_INTERFACE) and a local IP address. In doing so, it allows the user to work-around some serious
performance issues on machines with lots of virtual interfaces because curl no longer has to scan all interfaces each time it makes a connection.
This patch has clearly been neglected by me, and while I'm not quite ready to take it on just yet[*] I wanted to ask if you could consider pushing this to
github as a PR?
That way we'd get it tested and verified much better before merge, and it also helps out to tell us when it starts to get merge confliects (and it makes it
harder for us/me to forget it). Like it does now: the changes in lib/connect.c needs a rebase/edit to apply on current git master.
Further: I would like to get a clearer view on what exactly CURLOPT_LOCALADDR provides and offers that CURLOPT_INTERFACE prefixed with "host!" doesn't do? And
if so, can't we instead make CURLOPT_INTERFACE use that? Or perhaps even add another prefix for CURLOPT_INTERFACE if it truly has to be different. To me, this
feels like adding an entirely new easy option for a very minor variation of an already existing option.
[*] = I'll take off on an extended vacation now for a few weeks ahead so I personally won't work on anything larger until I get back.
Hello,

I just rebased, and here is the (untested) patch again. I'll be testing this, but since my normal
use case is pretty specific, and there were some tricky conflicts in the connect.c code,
it could use some other testing as well....

Thanks,
Ben
--
Ben Greear <***@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
Ben Greear via curl-library
2018-12-07 14:41:46 UTC
Permalink
Post by Daniel Stenberg
Hi Ben!
This allows a user to bind to both an interface (with CURLOPT_INTERFACE) and a local IP address. In doing so, it allows the user to work-around some serious performance issues on machines with lots of virtual interfaces because curl no longer has to scan all interfaces each time it makes a connection.
This patch has clearly been neglected by me, and while I'm not quite ready to take it on just yet[*] I wanted to ask if you could consider pushing this to github as a PR?
That way we'd get it tested and verified much better before merge, and it also helps out to tell us when it starts to get merge confliects (and it makes it harder for us/me to forget it). Like it does now: the changes in lib/connect.c needs a rebase/edit to apply on current git master.
Further: I would like to get a clearer view on what exactly CURLOPT_LOCALADDR provides and offers that CURLOPT_INTERFACE prefixed with "host!" doesn't do? And if so, can't we instead make CURLOPT_INTERFACE use that? Or perhaps even add another prefix for CURLOPT_INTERFACE if it truly has to be different. To me, this feels like adding an entirely new easy option for a very minor variation of an already existing option.
[*] = I'll take off on an extended vacation now for a few weeks ahead so I personally won't work on anything larger until I get back.
Hello,

I recently rebased this patch and did a new pull request (pull requests are not my specialty, possibly
I did that wrong).

Even if you don't want to take the other two compile fixup patches we recently discussed,
could you give consideration to the LOCALADDR patch?

Thanks,
Ben
--
Ben Greear <***@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
-------------------------------------------------------------------
Unsubscribe: https://cool.haxx.se/list/listinfo/curl-library
Etiquette: https://curl.haxx
Daniel Stenberg via curl-library
2018-12-07 14:52:30 UTC
Permalink
Post by Ben Greear via curl-library
I recently rebased this patch and did a new pull request (pull requests are
not my specialty, possibly I did that wrong).
Are you talking about https://github.com/curl/curl/pull/2177 ?

It still has compiler warnings:
https://travis-ci.org/curl/curl/jobs/460883263#L3654
Post by Ben Greear via curl-library
Even if you don't want to take the other two compile fixup patches we
recently discussed, could you give consideration to the LOCALADDR patch?
I replied regarding one of your suggested fixes earlier today. I made a PR out
of it.
--
/ daniel.haxx.se
-------------------------------------------------------------------
Unsubscribe: https://cool.haxx.se/list/listinfo/curl-library
Etiquette: https://curl.haxx.se
Ben Greear via curl-library
2018-12-07 15:11:54 UTC
Permalink
Post by Daniel Stenberg via curl-library
I recently rebased this patch and did a new pull request (pull requests are not my specialty, possibly I did that wrong).
Are you talking about https://github.com/curl/curl/pull/2177 ?
It still has compiler warnings: https://travis-ci.org/curl/curl/jobs/460883263#L3654
I didn't realize that... I just took a look at that link above and I'm not sure from the output how
to reproduce this build failure.

Do you know how I might reproduce this failure on my system, or do you see what
part of my patch is causing this issue? For what its worth, it builds clean on
all of my many systems...
Post by Daniel Stenberg via curl-library
Even if you don't want to take the other two compile fixup patches we recently discussed, could you give consideration to the LOCALADDR patch?
I replied regarding one of your suggested fixes earlier today. I made a PR out of it.
Thanks, I found that email...must have gotten up too early this morning!

Thanks,
Ben
--
Ben Greear <***@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
-------------------------------------------------------------------
Unsubscribe: https://cool.haxx.se/list/listinfo/curl-library
Etiquette:
Christian Hägele
2018-01-31 13:24:48 UTC
Permalink
Post by g***@candelatech.com
This allows a user to bind to both an interface (with CURLOPT_INTERFACE)
and a local IP address. In doing so, it allows the user to work-around
some serious performance issues on machines with lots of virtual interfaces
because curl no longer has to scan all interfaces each time it makes
a connection.
I'm very interested in this feature.
However, I think you missed one important topic. When you are using
DNS-Names you cannot know if it resolves to IPv4 or IPv6 or both. So if
you bind to a local IPv6 and the host only supports IPv4 connection will
fail. Same is true vice versa.
The only way I can think of solving this problem is by providing a local
IPv4 and a local IPv6 at the same time and let Curl choose after
DNS-Resolution which to use or maybe use both (for Happy-Eyeballs).

Any comments on this? (Disclaimer: I did not look at the code, only at the
documentation of the option)

Regards,

Christian
-------------------------------------------------------------------
Unsubscribe: https://cool.haxx.se/list/listinfo/curl-library
Etiquette: https://
Ben Greear
2018-01-31 15:25:50 UTC
Permalink
Post by Christian Hägele
Post by g***@candelatech.com
This allows a user to bind to both an interface (with CURLOPT_INTERFACE)
and a local IP address. In doing so, it allows the user to work-around
some serious performance issues on machines with lots of virtual interfaces
because curl no longer has to scan all interfaces each time it makes
a connection.
I'm very interested in this feature.
However, I think you missed one important topic. When you are using
DNS-Names you cannot know if it resolves to IPv4 or IPv6 or both. So if
you bind to a local IPv6 and the host only supports IPv4 connection will
fail. Same is true vice versa.
The only way I can think of solving this problem is by providing a local
IPv4 and a local IPv6 at the same time and let Curl choose after
DNS-Resolution which to use or maybe use both (for Happy-Eyeballs).
Any comments on this? (Disclaimer: I did not look at the code, only at the
documentation of the option)
There are already ways to bind to local IPs, my patch is adding a feature to bind
explicitly to an interface. This saves a large amount of time from the current feature,
which tries to figure out if a particular string is a device name or IP address.

Thanks,
Ben
Post by Christian Hägele
Regards,
Christian
-------------------------------------------------------------------
Unsubscribe: https://cool.haxx.se/list/listinfo/curl-library
Etiquette: https://curl.haxx.se/mail/etiquette.html
--
Ben Greear <***@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
-------------------------------------------------------------------
Unsubscribe: https://cool.haxx.se/list/listinfo/curl-library
Etiquette:
Ben Greear
2018-03-19 20:24:17 UTC
Permalink
Post by Ben Greear
Post by Christian Hägele
Post by g***@candelatech.com
This allows a user to bind to both an interface (with CURLOPT_INTERFACE)
and a local IP address. In doing so, it allows the user to work-around
some serious performance issues on machines with lots of virtual interfaces
because curl no longer has to scan all interfaces each time it makes
a connection.
I'm very interested in this feature.
However, I think you missed one important topic. When you are using
DNS-Names you cannot know if it resolves to IPv4 or IPv6 or both. So if
you bind to a local IPv6 and the host only supports IPv4 connection will
fail. Same is true vice versa.
The only way I can think of solving this problem is by providing a local
IPv4 and a local IPv6 at the same time and let Curl choose after
DNS-Resolution which to use or maybe use both (for Happy-Eyeballs).
Any comments on this? (Disclaimer: I did not look at the code, only at the
documentation of the option)
There are already ways to bind to local IPs, my patch is adding a feature to bind
explicitly to an interface. This saves a large amount of time from the current feature,
which tries to figure out if a particular string is a device name or IP address.
Hello,

Looks like it is the start of a release again...maybe a good time to revisit this
patch for inclusion?

My curl tree with the most recent version of my patch is here:

https://github.com/greearb/curl

Thanks,
Ben
--
Ben Greear <***@candelatech.com>
Candela Technologies Inc http://www.candelatech.com

-------------------------------------------------------------------
Unsubscribe: https://cool.haxx.se/list/listinfo/curl-library
E
Loading...