]> git.ipfire.org Git - thirdparty/haproxy.git/commitdiff
MAJOR: stream: store the target address into s->target_addr
authorWilly Tarreau <w@1wt.eu>
Thu, 18 Jul 2019 13:47:45 +0000 (15:47 +0200)
committerWilly Tarreau <w@1wt.eu>
Fri, 19 Jul 2019 11:50:09 +0000 (13:50 +0200)
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.

include/proto/backend.h
src/backend.c
src/cli.c
src/hlua.c
src/http_ana.c
src/peers.c

index 0e39d035c600d7cf67fa689a27afe9b8167124f7..97b11d64851416280069618479040ad6d83ea9e7 100644 (file)
@@ -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);
index a20150a3e8b3a3bfd604b6ba938343ba7713a1cc..1002425a28c341f3379ed517a81fb762d11de67a 100644 (file)
@@ -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;
                }
 
index ce33f60a3f2d236ae7e7fb86b1b32b45e673277c..ab4d3690aeb44c0dc30655ca3807b14cc9e6aa14 100644 (file)
--- 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 */
index 49f056c3988cc67b80ee0c350988a45cdb07ebae..3b58dc037cd5108f214e5feaf6a2cf91fd9ab192 100644 (file)
@@ -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);
 
index 782c1689df5e3113a0ec43420453e4a5722117d0..504f98a91dffbda8139f4454bee0426f02978d0b 100644 (file)
@@ -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;
        }
 
        /*
index 8fa6a8e4a71cbe62a795c4bae4ffba35968f680b..6d66553c65b64a483df677adf4bb380f1b5cd1fa 100644 (file)
@@ -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;