You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by zw...@apache.org on 2013/08/30 18:20:36 UTC
[23/50] git commit: TS-1424: Fix KeepAlive 4-tuple collision for
transparent proxy
TS-1424: Fix KeepAlive 4-tuple collision for transparent proxy
Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo
Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/f63b27d5
Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/f63b27d5
Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/f63b27d5
Branch: refs/heads/5.0.x
Commit: f63b27d5220c95cd95764fb0ca0cb4e648b2f055
Parents: 14c9cf2
Author: Alan M. Carroll <am...@network-geographics.com>
Authored: Fri Aug 23 10:44:08 2013 -0500
Committer: Alan M. Carroll <am...@network-geographics.com>
Committed: Fri Aug 23 22:35:51 2013 -0500
----------------------------------------------------------------------
proxy/http/HttpSM.cc | 43 +++++++++++++++++++++++++++++++++--------
proxy/http/HttpTransact.cc | 33 +++++++++++++++++--------------
proxy/http/HttpTransact.h | 20 ++++++++++++-------
3 files changed, 66 insertions(+), 30 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/f63b27d5/proxy/http/HttpSM.cc
----------------------------------------------------------------------
diff --git a/proxy/http/HttpSM.cc b/proxy/http/HttpSM.cc
index 92bd92c..ab120db 100644
--- a/proxy/http/HttpSM.cc
+++ b/proxy/http/HttpSM.cc
@@ -1712,7 +1712,34 @@ HttpSM::state_http_server_open(int event, void *data)
case VC_EVENT_ERROR:
case NET_EVENT_OPEN_FAILED:
t_state.current.state = HttpTransact::CONNECTION_ERROR;
- call_transact_and_set_next_state(HttpTransact::HandleResponse);
+ // save the errno from the connect fail for future use (passed as negative value, flip back)
+ t_state.current.server->set_connect_fail(event == NET_EVENT_OPEN_FAILED ? -reinterpret_cast<intptr_t>(data) : EREMOTEIO);
+
+ /* If we get this error, then we simply can't bind to the 4-tuple to make the connection. There's no hope of
+ retries succeeding in the near future. The best option is to just shut down the connection without further
+ comment. The only known cause for this is outbound transparency combined with use client target address / source
+ port, as noted in TS-1424. If the keep alives desync the current connection can be attempting to rebind the 4
+ tuple simultaneously with the shut down of an existing connection. Dropping the client side will cause it to pick
+ a new source port and recover from this issue.
+ */
+ if (EADDRNOTAVAIL == t_state.current.server->connect_result) {
+ if (is_debug_tag_set("http_tproxy")) {
+ ip_port_text_buffer ip_c, ip_s;
+ Debug("http_tproxy", "Force close of client connect (%s->%s) due to EADDRNOTAVAIL [%" PRId64 "]"
+ , ats_ip_nptop(&t_state.client_info.addr.sa, ip_c, sizeof ip_c)
+ , ats_ip_nptop(&t_state.server_info.addr.sa, ip_s, sizeof ip_s)
+ , sm_id
+ );
+ }
+ t_state.client_info.keep_alive = HTTP_NO_KEEPALIVE; // part of the problem, clear it.
+ if (ua_entry && ua_entry->vc) {
+ vc_table.cleanup_entry(ua_entry);
+ ua_entry = NULL;
+ ua_session = NULL;
+ }
+ } else {
+ call_transact_and_set_next_state(HttpTransact::HandleResponse);
+ }
return 0;
case CONGESTION_EVENT_CONGESTED_ON_F:
t_state.current.state = HttpTransact::CONGEST_CONTROL_CONGESTED_ON_F;
@@ -2048,7 +2075,6 @@ HttpSM::process_hostdb_info(HostDBInfo * r)
// current time when calling select_best_http().
HostDBRoundRobin *rr = r->rr();
ret = rr->select_best_http(&t_state.client_info.addr.sa, now, (int) t_state.txn_conf->down_server_timeout);
- t_state.dns_info.round_robin = true;
// set the srv target`s last_failure
if (t_state.dns_info.srv_lookup_success) {
@@ -2068,7 +2094,6 @@ HttpSM::process_hostdb_info(HostDBInfo * r)
}
} else {
ret = r;
- t_state.dns_info.round_robin = false;
}
if (ret) {
t_state.host_db_info = *ret;
@@ -2079,9 +2104,9 @@ HttpSM::process_hostdb_info(HostDBInfo * r)
DebugSM("http", "[%" PRId64 "] DNS lookup failed for '%s'", sm_id, t_state.dns_info.lookup_name);
t_state.dns_info.lookup_success = false;
- t_state.dns_info.round_robin = false;
t_state.host_db_info.app.allotment.application1 = 0;
t_state.host_db_info.app.allotment.application2 = 0;
+ ink_assert(!t_state.host_db_info.round_robin);
}
milestones.dns_lookup_end = ink_get_hrtime();
@@ -2923,10 +2948,12 @@ HttpSM::tunnel_handler_server(int event, HttpTunnelProducer * p)
p->read_vio = NULL;
/* TS-1424: if we're outbound transparent and using the client
source port for the outbound connection we must effectively
- propagate server closes back to the client.
+ propagate server closes back to the client. Part of that is
+ disabling KeepAlive if the server closes.
*/
- if (ua_session && ua_session->f_outbound_transparent && t_state.http_config_param->use_client_source_port)
+ if (ua_session && ua_session->f_outbound_transparent && t_state.http_config_param->use_client_source_port) {
t_state.client_info.keep_alive = HTTP_NO_KEEPALIVE;
+ }
} else {
server_session->attach_hostname(t_state.current.server->name);
server_session->server_trans_stat--;
@@ -3991,7 +4018,7 @@ HttpSM::do_hostdb_update_if_necessary()
t_state.updated_server_version = HostDBApplicationInfo::HTTP_VERSION_UNDEFINED;
}
// Check to see if we need to report or clear a connection failure
- if (t_state.current.server->connect_failure) {
+ if (t_state.current.server->had_connect_fail()) {
issue_update |= 1;
mark_host_failure(&t_state.host_db_info, t_state.client_request_time);
} else {
@@ -4887,7 +4914,7 @@ HttpSM::mark_server_down_on_client_abort()
wait = 0;
}
if (ink_hrtime_to_sec(wait) > t_state.txn_conf->client_abort_threshold) {
- t_state.current.server->connect_failure = true;
+ t_state.current.server->set_connect_fail(ETIMEDOUT);
do_hostdb_update_if_necessary();
}
}
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/f63b27d5/proxy/http/HttpTransact.cc
----------------------------------------------------------------------
diff --git a/proxy/http/HttpTransact.cc b/proxy/http/HttpTransact.cc
index ac074ed..e39e7e9 100644
--- a/proxy/http/HttpTransact.cc
+++ b/proxy/http/HttpTransact.cc
@@ -1408,7 +1408,7 @@ HttpTransact::PPDNSLookup(State* s)
// lookup succeeded, open connection to p.p.
ats_ip_copy(&s->parent_info.addr, s->host_db_info.ip());
get_ka_info_from_host_db(s, &s->parent_info, &s->client_info, &s->host_db_info);
- s->parent_info.dns_round_robin = s->dns_info.round_robin;
+ s->parent_info.dns_round_robin = s->host_db_info.round_robin;
char addrbuf[INET6_ADDRSTRLEN];
DebugTxn("http_trans", "[PPDNSLookup] DNS lookup for sm_id[%" PRId64"] successful IP: %s",
@@ -1452,19 +1452,19 @@ void
HttpTransact::ReDNSRoundRobin(State* s)
{
ink_assert(s->current.server == &s->server_info);
- ink_assert(s->current.server->connect_failure);
+ ink_assert(s->current.server->had_connect_fail());
if (s->dns_info.lookup_success) {
// We using a new server now so clear the connection
// failure mark
- s->current.server->connect_failure = 0;
+ s->current.server->clear_connect_fail();
// Our ReDNS of the server succeeeded so update the necessary
// information and try again
ats_ip_copy(&s->server_info.addr, s->host_db_info.ip());
ats_ip_copy(&s->request_data.dest_ip, &s->server_info.addr);
get_ka_info_from_host_db(s, &s->server_info, &s->client_info, &s->host_db_info);
- s->server_info.dns_round_robin = s->dns_info.round_robin;
+ s->server_info.dns_round_robin = s->host_db_info.round_robin;
char addrbuf[INET6_ADDRSTRLEN];
DebugTxn("http_trans", "[ReDNSRoundRobin] DNS lookup for O.S. successful IP: %s",
@@ -1612,7 +1612,7 @@ HttpTransact::OSDNSLookup(State* s)
// We've backed off from a client supplied address and found some
// HostDB addresses. We use those if they're different from the CTA.
// In all cases we now commit to client or HostDB for our source.
- if (s->dns_info.round_robin) {
+ if (s->host_db_info.round_robin) {
HostDBInfo* cta = s->host_db_info.rr()->select_next(&s->current.server->addr.sa);
if (cta) {
// found another addr, lock in host DB.
@@ -1635,7 +1635,7 @@ HttpTransact::OSDNSLookup(State* s)
ats_ip_copy(&s->server_info.addr, s->host_db_info.ip());
ats_ip_copy(&s->request_data.dest_ip, &s->server_info.addr);
get_ka_info_from_host_db(s, &s->server_info, &s->client_info, &s->host_db_info);
- s->server_info.dns_round_robin = s->dns_info.round_robin;
+ s->server_info.dns_round_robin = s->host_db_info.round_robin;
char addrbuf[INET6_ADDRSTRLEN];
DebugTxn("http_trans", "[OSDNSLookup] DNS lookup for O.S. successful "
@@ -3356,7 +3356,7 @@ HttpTransact::handle_response_from_parent(State* s)
switch (s->current.state) {
case CONNECTION_ALIVE:
DebugTxn("http_trans", "[hrfp] connection alive");
- s->current.server->connect_failure = 0;
+ s->current.server->connect_result = 0;
SET_VIA_STRING(VIA_DETAIL_PP_CONNECT, VIA_DETAIL_PP_SUCCESS);
if (s->parent_result.retry) {
s->parent_params->recordRetrySuccess(&s->parent_result);
@@ -3371,7 +3371,7 @@ HttpTransact::handle_response_from_parent(State* s)
ink_assert(s->hdr_info.server_request.valid());
- s->current.server->connect_failure = 1;
+ s->current.server->connect_result = ENOTCONN;
char addrbuf[INET6_ADDRSTRLEN];
DebugTxn("http_trans", "[%d] failed to connect to parent %s", s->current.attempts,
@@ -3481,14 +3481,14 @@ HttpTransact::handle_response_from_server(State* s)
case CONNECTION_ALIVE:
DebugTxn("http_trans", "[hrfs] connection alive");
SET_VIA_STRING(VIA_DETAIL_SERVER_CONNECT, VIA_DETAIL_SERVER_SUCCESS);
- s->current.server->connect_failure = 0;
+ s->current.server->clear_connect_fail();
handle_forward_server_connection_open(s);
break;
case CONGEST_CONTROL_CONGESTED_ON_F:
case CONGEST_CONTROL_CONGESTED_ON_M:
DebugTxn("http_trans", "[handle_response_from_server] Error. congestion control -- congested.");
SET_VIA_STRING(VIA_DETAIL_SERVER_CONNECT, VIA_DETAIL_SERVER_FAILURE);
- s->current.server->connect_failure = 1;
+ s->current.server->set_connect_fail(EUSERS); // too many users
handle_server_connection_not_open(s);
break;
case OPEN_RAW_ERROR:
@@ -3504,7 +3504,10 @@ HttpTransact::handle_response_from_server(State* s)
case CONNECTION_CLOSED:
/* fall through */
case BAD_INCOMING_RESPONSE:
- s->current.server->connect_failure = 1;
+ // Set to generic I/O error if not already set specifically.
+ if (!s->current.server->had_connect_fail())
+ s->current.server->set_connect_fail(EIO);
+
if (is_server_negative_cached(s)) {
max_connect_retries = s->txn_conf->connect_attempts_max_retries_dead_server;
} else {
@@ -3549,7 +3552,7 @@ HttpTransact::handle_response_from_server(State* s)
case ACTIVE_TIMEOUT:
DebugTxn("http_trans", "[hrfs] connection not alive");
SET_VIA_STRING(VIA_DETAIL_SERVER_CONNECT, VIA_DETAIL_SERVER_FAILURE);
- s->current.server->connect_failure = 1;
+ s->current.server->set_connect_fail(ETIMEDOUT);
handle_server_connection_not_open(s);
break;
default:
@@ -3582,7 +3585,7 @@ HttpTransact::delete_server_rr_entry(State* s, int max_retries)
DebugTxn("http_trans", "[%d] failed to connect to %s", s->current.attempts,
ats_ip_ntop(&s->current.server->addr.sa, addrbuf, sizeof(addrbuf)));
DebugTxn("http_trans", "[delete_server_rr_entry] marking rr entry " "down and finding next one");
- ink_assert(s->current.server->connect_failure);
+ ink_assert(s->current.server->had_connect_fail());
ink_assert(s->current.request_to == ORIGIN_SERVER);
ink_assert(s->current.server == &s->server_info);
update_dns_info(&s->dns_info, &s->current, 0, &s->arena);
@@ -3609,7 +3612,7 @@ HttpTransact::retry_server_connection_not_open(State* s, ServerState_t conn_stat
ink_assert(s->current.state != CONNECTION_ALIVE);
ink_assert(s->current.state != ACTIVE_TIMEOUT);
ink_assert(s->current.attempts <= max_retries);
- ink_assert(s->current.server->connect_failure != 0);
+ ink_assert(s->current.server->had_connect_fail());
char addrbuf[INET6_ADDRSTRLEN];
char *url_string = s->hdr_info.client_request.url_string_get(&s->arena);
@@ -3660,7 +3663,7 @@ HttpTransact::handle_server_connection_not_open(State* s)
DebugTxn("http_trans", "[handle_server_connection_not_open] (hscno)");
DebugTxn("http_seq", "[HttpTransact::handle_server_connection_not_open] ");
ink_assert(s->current.state != CONNECTION_ALIVE);
- ink_assert(s->current.server->connect_failure != 0);
+ ink_assert(s->current.server->had_connect_fail());
SET_VIA_STRING(VIA_SERVER_RESULT, VIA_SERVER_ERROR);
HTTP_INCREMENT_TRANS_STAT(http_broken_server_connections_stat);
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/f63b27d5/proxy/http/HttpTransact.h
----------------------------------------------------------------------
diff --git a/proxy/http/HttpTransact.h b/proxy/http/HttpTransact.h
index 27e4ffb..6a66fd1 100644
--- a/proxy/http/HttpTransact.h
+++ b/proxy/http/HttpTransact.h
@@ -677,7 +677,7 @@ public:
{ }
} RedirectInfo;
- typedef struct _ConnectionAttributes
+ struct ConnectionAttributes
{
HTTPVersion http_version;
HTTPKeepAlive keep_alive;
@@ -687,9 +687,11 @@ public:
bool receive_chunked_response;
bool pipeline_possible;
bool proxy_connect_hdr;
+ /// @c errno from the most recent attempt to connect.
+ /// zero means no failure (not attempted, succeeded).
+ int connect_result;
char *name;
bool dns_round_robin;
- bool connect_failure;
TransferEncoding_t transfer_encoding;
IpEndpoint addr; // replaces 'ip' field
@@ -709,15 +711,19 @@ public:
/// @c true if the connection is transparent.
bool is_transparent;
- _ConnectionAttributes()
+ bool had_connect_fail() const { return 0 != connect_result; }
+ void clear_connect_fail() { connect_result = 0; }
+ void set_connect_fail(int e) { connect_result = e; }
+
+ ConnectionAttributes()
: http_version(),
keep_alive(HTTP_KEEPALIVE_UNDEFINED),
receive_chunked_response(false),
pipeline_possible(false),
proxy_connect_hdr(false),
+ connect_result(0),
name(NULL),
dns_round_robin(false),
- connect_failure(false),
transfer_encoding(NO_TRANSFER_ENCODING),
port(0),
state(STATE_UNDEFINED),
@@ -727,7 +733,8 @@ public:
{
memset(&addr, 0, sizeof(addr));
}
- } ConnectionAttributes;
+
+ };
typedef struct _CurrentInfo
{
@@ -770,14 +777,13 @@ public:
char *lookup_name;
char srv_hostname[MAXDNAME];
LookingUp_t looking_up;
- bool round_robin;
bool srv_lookup_success;
short srv_port;
HostDBApplicationInfo srv_app;
_DNSLookupInfo()
: attempts(0), os_addr_style(OS_ADDR_TRY_DEFAULT),
- lookup_success(false), lookup_name(NULL), looking_up(UNDEFINED_LOOKUP), round_robin(false),
+ lookup_success(false), lookup_name(NULL), looking_up(UNDEFINED_LOOKUP),
srv_lookup_success(false), srv_port(0)
{
srv_hostname[0] = '\0';