From: Willy Tarreau <w@1wt.eu>
Date: Thu, 18 Jul 2019 13:47:45 +0000 (+0200)
Subject: MAJOR: stream: store the target address into s->target_addr
X-Git-Tag: v2.1-dev2~295
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=1c8d32bb6296f15e7416f1fa91aaeaf6c9fc2075;p=thirdparty%2Fhaproxy.git

MAJOR: stream: store the target address into s->target_addr

When forcing the outgoing address of a connection, till now we used to
allocate this outgoing connection and set the address into it, then set
SF_ADDR_SET. With connection reuse this causes a whole lot of issues and
difficulties in the code.

Thanks to the previous changes, it is now possible to store the target
address into the stream instead, and copy the address from the stream to
the connection when initializing the connection. assign_server_address()
does this and as a result SF_ADDR_SET now reflects the presence of the
target address in the stream, not in the connection. The http_proxy mode,
the peers and the master's CLI now use the same mechanism. For now the
existing connection code was not removed to limit the amount of tricky
changes, but the allocated connection is not used anymore.

This change also revealed a latent issue that we've been having around
option http_proxy : the address was set in the connection but neither the
SF_ADDR_SET nor the SF_ASSIGNED flags were set. It looks like the connection
could establish only due to the fact that it existed with a non-null
destination address.
---

diff --git a/include/proto/backend.h b/include/proto/backend.h
index 0e39d035c6..97b11d6485 100644
--- a/include/proto/backend.h
+++ b/include/proto/backend.h
@@ -31,7 +31,7 @@
 #include <types/stream.h>
 
 int assign_server(struct stream *s);
-int assign_server_address(struct stream *s, struct connection *srv_conn);
+int assign_server_address(struct stream *s);
 int assign_server_and_queue(struct stream *s);
 int connect_server(struct stream *s);
 int srv_redispatch_connect(struct stream *t);
diff --git a/src/backend.c b/src/backend.c
index a20150a3e8..1002425a28 100644
--- a/src/backend.c
+++ b/src/backend.c
@@ -815,18 +815,14 @@ out_ok:
  *
  * Upon successful return, the stream flag SF_ADDR_SET is set. This flag is
  * not cleared, so it's to the caller to clear it if required.
- *
- * The caller is responsible for having already assigned a connection
- * to si->end.
- *
  */
-int assign_server_address(struct stream *s, struct connection *srv_conn)
+int assign_server_address(struct stream *s)
 {
 	struct connection *cli_conn = objt_conn(strm_orig(s));
 
 	DPRINTF(stderr,"assign_server_address : s=%p\n",s);
 
-	if (!sockaddr_alloc(&srv_conn->dst))
+	if (!sockaddr_alloc(&s->target_addr))
 		return SRV_STATUS_INTERNAL;
 
 	if ((s->flags & SF_DIRECT) || (s->be->lbprm.algo & BE_LB_KIND)) {
@@ -834,10 +830,10 @@ int assign_server_address(struct stream *s, struct connection *srv_conn)
 		if (!(s->flags & SF_ASSIGNED))
 			return SRV_STATUS_INTERNAL;
 
-		*srv_conn->dst = __objt_server(s->target)->addr;
-		set_host_port(srv_conn->dst, __objt_server(s->target)->svc_port);
+		*s->target_addr = __objt_server(s->target)->addr;
+		set_host_port(s->target_addr, __objt_server(s->target)->svc_port);
 
-		if (!is_addr(srv_conn->dst) && cli_conn) {
+		if (!is_addr(s->target_addr) && cli_conn) {
 			/* if the server has no address, we use the same address
 			 * the client asked, which is handy for remapping ports
 			 * locally on multiple addresses at once. Nothing is done
@@ -846,9 +842,9 @@ int assign_server_address(struct stream *s, struct connection *srv_conn)
 			if (!conn_get_dst(cli_conn)) {
 				/* do nothing if we can't retrieve the address */
 			} else if (cli_conn->dst->ss_family == AF_INET) {
-				((struct sockaddr_in *)srv_conn->dst)->sin_addr = ((struct sockaddr_in *)cli_conn->dst)->sin_addr;
+				((struct sockaddr_in *)s->target_addr)->sin_addr = ((struct sockaddr_in *)cli_conn->dst)->sin_addr;
 			} else if (cli_conn->dst->ss_family == AF_INET6) {
-				((struct sockaddr_in6 *)srv_conn->dst)->sin6_addr = ((struct sockaddr_in6 *)cli_conn->dst)->sin6_addr;
+				((struct sockaddr_in6 *)s->target_addr)->sin6_addr = ((struct sockaddr_in6 *)cli_conn->dst)->sin6_addr;
 			}
 		}
 
@@ -862,20 +858,20 @@ int assign_server_address(struct stream *s, struct connection *srv_conn)
 				base_port = get_host_port(cli_conn->dst);
 
 				/* Second, assign the outgoing connection's port */
-				base_port += get_host_port(srv_conn->dst);
-				set_host_port(srv_conn->dst, base_port);
+				base_port += get_host_port(s->target_addr);
+				set_host_port(s->target_addr, base_port);
 			}
 		}
 	}
 	else if (s->be->options & PR_O_DISPATCH) {
 		/* connect to the defined dispatch addr */
-		*srv_conn->dst = s->be->dispatch_addr;
+		*s->target_addr = s->be->dispatch_addr;
 	}
 	else if ((s->be->options & PR_O_TRANSP) && cli_conn) {
 		/* in transparent mode, use the original dest addr if no dispatch specified */
 		if (conn_get_dst(cli_conn) &&
 		    (cli_conn->dst->ss_family == AF_INET || cli_conn->dst->ss_family == AF_INET6))
-			*srv_conn->dst = *cli_conn->dst;
+			*s->target_addr = *cli_conn->dst;
 	}
 	else if (s->be->options & PR_O_HTTP_PROXY) {
 		/* If HTTP PROXY option is set, then server is already assigned
@@ -886,9 +882,6 @@ int assign_server_address(struct stream *s, struct connection *srv_conn)
 		return SRV_STATUS_INTERNAL;
 	}
 
-	/* Copy network namespace from client connection */
-	srv_conn->proxy_netns = cli_conn ? cli_conn->proxy_netns : NULL;
-
 	s->flags |= SF_ADDR_SET;
 	return SRV_STATUS_OK;
 }
@@ -1146,7 +1139,7 @@ fail:
  */
 int connect_server(struct stream *s)
 {
-	struct connection *cli_conn = NULL;
+	struct connection *cli_conn = objt_conn(strm_orig(s));
 	struct connection *srv_conn = NULL;
 	struct connection *old_conn = NULL;
 	struct conn_stream *srv_cs = NULL;
@@ -1417,15 +1410,21 @@ int connect_server(struct stream *s)
 		}
 	}
 
-	if (!srv_conn)
+	if (!srv_conn || !sockaddr_alloc(&srv_conn->dst))
 		return SF_ERR_RESOURCE;
 
 	if (!(s->flags & SF_ADDR_SET)) {
-		err = assign_server_address(s, srv_conn);
+		err = assign_server_address(s);
 		if (err != SRV_STATUS_OK)
 			return SF_ERR_INTERNAL;
 	}
 
+	/* copy the target address into the connection */
+	*srv_conn->dst = *s->target_addr;
+
+	/* Copy network namespace from client connection */
+	srv_conn->proxy_netns = cli_conn ? cli_conn->proxy_netns : NULL;
+
 	if (!conn_xprt_ready(srv_conn) && !srv_conn->mux) {
 		/* set the correct protocol on the output stream interface */
 		if (srv)
@@ -1468,11 +1467,8 @@ int connect_server(struct stream *s)
 		}
 
 #endif
-
-
 		/* process the case where the server requires the PROXY protocol to be sent */
 		srv_conn->send_proxy_ofs = 0;
-		cli_conn = objt_conn(strm_orig(s));
 
 		if (srv && srv->pp_opts) {
 			srv_conn->flags |= CO_FL_PRIVATE;
@@ -1631,6 +1627,7 @@ int srv_redispatch_connect(struct stream *s)
 		if (((s->flags & (SF_DIRECT|SF_FORCE_PRST)) == SF_DIRECT) &&
 		    (s->be->options & PR_O_REDISP)) {
 			s->flags &= ~(SF_DIRECT | SF_ASSIGNED | SF_ADDR_SET);
+			sockaddr_free(&s->target_addr);
 			goto redispatch;
 		}
 
diff --git a/src/cli.c b/src/cli.c
index ce33f60a3f..ab4d3690ae 100644
--- a/src/cli.c
+++ b/src/cli.c
@@ -2319,6 +2319,8 @@ int pcli_wait_for_response(struct stream *s, struct channel *rep, int an_bit)
 			s->srv_conn = NULL;
 		}
 
+		sockaddr_free(&s->target_addr);
+
 		s->si[1].state     = s->si[1].prev_state = SI_ST_INI;
 		s->si[1].err_type  = SI_ET_NONE;
 		s->si[1].conn_retries = 0;  /* used for logging too */
diff --git a/src/hlua.c b/src/hlua.c
index 49f056c398..3b58dc037c 100644
--- a/src/hlua.c
+++ b/src/hlua.c
@@ -2440,6 +2440,8 @@ __LJMP static int hlua_socket_connect(struct lua_State *L)
 	si = appctx->owner;
 	s = si_strm(si);
 
+	/* FIXME WTA: the conn-specific code below should now be useless */
+
 	/* Initialise connection. */
 	conn = cs_conn(si_alloc_cs(&s->si[1], NULL));
 	if (!conn) {
@@ -2461,30 +2463,36 @@ __LJMP static int hlua_socket_connect(struct lua_State *L)
 		WILL_LJMP(luaL_error(L, "connect: port ranges not supported : address '%s'", ip));
 	}
 
-	if (!sockaddr_alloc(&conn->dst)) {
-		xref_unlock(&socket->xref, peer);
-		WILL_LJMP(luaL_error(L, "connect: internal error"));
-	}
-
-	memcpy(conn->dst, addr, sizeof(struct sockaddr_storage));
-
 	/* Set port. */
 	if (low == 0) {
-		if (conn->dst->ss_family == AF_INET) {
+		if (addr->ss_family == AF_INET) {
 			if (port == -1) {
 				xref_unlock(&socket->xref, peer);
 				WILL_LJMP(luaL_error(L, "connect: port missing"));
 			}
-			((struct sockaddr_in *)conn->dst)->sin_port = htons(port);
-		} else if (conn->dst->ss_family == AF_INET6) {
+			((struct sockaddr_in *)addr)->sin_port = htons(port);
+		} else if (addr->ss_family == AF_INET6) {
 			if (port == -1) {
 				xref_unlock(&socket->xref, peer);
 				WILL_LJMP(luaL_error(L, "connect: port missing"));
 			}
-			((struct sockaddr_in6 *)conn->dst)->sin6_port = htons(port);
+			((struct sockaddr_in6 *)addr)->sin6_port = htons(port);
 		}
 	}
 
+	if (!sockaddr_alloc(&conn->dst)) {
+		xref_unlock(&socket->xref, peer);
+		WILL_LJMP(luaL_error(L, "connect: internal error"));
+	}
+
+	memcpy(conn->dst, addr, sizeof(struct sockaddr_storage));
+
+	if (!sockaddr_alloc(&s->target_addr)) {
+		xref_unlock(&socket->xref, peer);
+		WILL_LJMP(luaL_error(L, "connect: internal error"));
+	}
+	*s->target_addr = *addr;
+
 	hlua = hlua_gethlua(L);
 	appctx = __objt_appctx(s->si[0].end);
 
diff --git a/src/http_ana.c b/src/http_ana.c
index 782c1689df..504f98a91d 100644
--- a/src/http_ana.c
+++ b/src/http_ana.c
@@ -732,17 +732,13 @@ int http_process_request(struct stream *s, struct channel *req, int an_bit)
 
 	/*
 	 * If HTTP PROXY is set we simply get remote server address parsing
-	 * incoming request. Note that this requires that a connection is
-	 * allocated on the server side.
+	 * incoming request.
 	 */
 	if ((s->be->options & PR_O_HTTP_PROXY) && !(s->flags & SF_ADDR_SET)) {
-		struct connection *conn;
 		struct htx_sl *sl;
 		struct ist uri, path;
 
-		/* Note that for now we don't reuse existing proxy connections */
-		if (unlikely((conn = cs_conn(si_alloc_cs(&s->si[1], NULL))) == NULL ||
-			     !sockaddr_alloc(&conn->dst))) {
+		if (!sockaddr_alloc(&s->target_addr)) {
 			txn->req.err_state = txn->req.msg_state;
 			txn->req.msg_state = HTTP_MSG_ERROR;
 			txn->status = 500;
@@ -760,9 +756,12 @@ int http_process_request(struct stream *s, struct channel *req, int an_bit)
 		uri = htx_sl_req_uri(sl);
 		path = http_get_path(uri);
 
-		if (url2sa(uri.ptr, uri.len - path.len, conn->dst, NULL) == -1)
+		if (url2sa(uri.ptr, uri.len - path.len, s->target_addr, NULL) == -1)
 			goto return_bad_req;
 
+		s->target = &s->be->obj_type;
+		s->flags |= SF_ADDR_SET | SF_ASSIGNED;
+
 		/* if the path was found, we have to remove everything between
 		 * uri.ptr and path.ptr (excluded). If it was not found, we need
 		 * to replace from all the uri by a single "/".
@@ -772,7 +771,6 @@ int http_process_request(struct stream *s, struct channel *req, int an_bit)
 		 * insignificant.
 		 */
 		istcpy(&uri, (path.len ? path : ist("/")), uri.len);
-		conn->target = &s->be->obj_type;
 	}
 
 	/*
diff --git a/src/peers.c b/src/peers.c
index 8fa6a8e4a7..6d66553c65 100644
--- a/src/peers.c
+++ b/src/peers.c
@@ -2525,9 +2525,15 @@ static struct appctx *peer_session_create(struct peers *peers, struct peer *peer
 	appctx_wakeup(appctx);
 
 	/* initiate an outgoing connection */
+	s->target = peer_session_target(peer, s);
+	if (!sockaddr_alloc(&s->target_addr))
+		goto out_free_strm;
+	*s->target_addr = peer->addr;
 	s->si[1].flags |= SI_FL_NOLINGER;
 	si_set_state(&s->si[1], SI_ST_ASS);
 
+	/* FIXME WTA: the connection below should now be totally useless */
+
 	/* automatically prepare the stream interface to connect to the
 	 * pre-initialized connection in si->conn.
 	 */
@@ -2537,7 +2543,7 @@ static struct appctx *peer_session_create(struct peers *peers, struct peer *peer
 	if (unlikely((cs = cs_new(conn)) == NULL))
 		goto out_free_conn;
 
-	conn->target = s->target = peer_session_target(peer, s);
+	conn->target = s->target;
 
 	if (!sockaddr_alloc(&conn->dst))
 		goto out_free_cs;