MEDIUM: stream: remove the confusing SF_ADDR_SET flag

This flag is no longer needed now that it must always match the presence
of a destination address on the backend conn_stream. Worse, before previous
patch, if it were to be accidently removed while the address is present, it
could result in a leak of that address since alloc_dst_address() would first
be called to flush it.

Its usage has a long history where addresses were stored in an area shared
with the connection, but as this is no longer the case, there's no reason
for putting this burden onto application-level code that should not focus
on setting obscure flags.

The only place where that made a small difference is in the dequeuing code
in case of queue redistribution, because previously the code would first
clear the flag, and only later when trying to deal with the queue, would
release the address. It's not even certain whether there would exist a
code path going to connect_server() without calling pendconn_dequeue()
first (e.g. retries on queue timeout maybe?).

Now the pendconn_dequeue() code will rely on SF_ASSIGNED to decide to
clear and release the address, since that flag is always set while in
a server's queue, and its clearance implies that we don't want to keep
the address. At least it remains consistent and there's no more risk of
leaking it.

diff --git a/dev/flags/flags.c b/dev/flags/flags.c
index 8052e8d40bfe43a7a973c7e6dc3a71ced64f8dd2..3a2b48ab9df2334e4b2b561e6190d17c1c0bb32d 100644 (file)
--- a/dev/flags/flags.c
+++ b/dev/flags/flags.c
@@ -402,7 +402,6 @@ void show_strm_flags(unsigned int f)
        SHOW_FLAG(f, SF_MONITOR);
        SHOW_FLAG(f, SF_FORCE_PRST);
        SHOW_FLAG(f, SF_BE_ASSIGNED);
-       SHOW_FLAG(f, SF_ADDR_SET);
        SHOW_FLAG(f, SF_ASSIGNED);
        SHOW_FLAG(f, SF_DIRECT);
 

diff --git a/include/haproxy/stream-t.h b/include/haproxy/stream-t.h
index 5e3541fafa70b89b12e95c88be85f984ce8952a9..4a017f4cd0a1c47bc6b2ce7719c796f8ff252dd4 100644 (file)
--- a/include/haproxy/stream-t.h
+++ b/include/haproxy/stream-t.h
@@ -37,7 +37,7 @@
 /* Various Stream Flags, bits values 0x01 to 0x100 (shift 0) */
 #define SF_DIRECT      0x00000001      /* connection made on the server matching the client cookie */
 #define SF_ASSIGNED    0x00000002      /* no need to assign a server to this stream */
-#define SF_ADDR_SET    0x00000004      /* this stream's server address has been set */
+/* unused: 0x00000004 */
 #define SF_BE_ASSIGNED 0x00000008      /* a backend was assigned. Conns are accounted. */
 
 #define SF_FORCE_PRST  0x00000010      /* force persistence here, even if server is down */

diff --git a/include/haproxy/stream.h b/include/haproxy/stream.h
index b77c9930f24be33506587d9c4c7d1d92fcdaaf2f..5660857d426698623137e7dce142c7060c17bbf8 100644 (file)
--- a/include/haproxy/stream.h
+++ b/include/haproxy/stream.h
@@ -339,7 +339,7 @@ static inline void stream_choose_redispatch(struct stream *s)
                        process_srv_queue(objt_server(s->target));
 
                sockaddr_free(&s->csb->dst);
-               s->flags &= ~(SF_DIRECT | SF_ASSIGNED | SF_ADDR_SET);
+               s->flags &= ~(SF_DIRECT | SF_ASSIGNED);
                s->csb->state = CS_ST_REQ;
        } else {
                if (objt_server(s->target))

diff --git a/src/backend.c b/src/backend.c
index c58445361bb192b3296d92efddb66664667b49f9..3b0e231c24aebc4349326f0588e92a551f0da768 100644 (file)
--- a/src/backend.c
+++ b/src/backend.c
@@ -1349,13 +1349,9 @@ static int connect_server(struct stream *s)
         * it can be NULL for dispatch mode or transparent backend */
        srv = objt_server(s->target);
 
-       if (!(s->flags & SF_ADDR_SET)) {
-               err = alloc_dst_address(&s->csb->dst, srv, s);
-               if (err != SRV_STATUS_OK)
-                       return SF_ERR_INTERNAL;
-
-               s->flags |= SF_ADDR_SET;
-       }
+       err = alloc_dst_address(&s->csb->dst, srv, s);
+       if (err != SRV_STATUS_OK)
+               return SF_ERR_INTERNAL;
 
        err = alloc_bind_address(&bind_addr, srv, s);
        if (err != SRV_STATUS_OK)
@@ -1904,7 +1900,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);
+                       s->flags &= ~(SF_DIRECT | SF_ASSIGNED);
                        sockaddr_free(&s->csb->dst);
                        goto redispatch;
                }

diff --git a/src/cli.c b/src/cli.c
index b32b8b1a4b62d88fa3e615980bf36b6c73507ad3..7f6b58b4efeb863513be6938d4371b5dad29d6a6 100644 (file)
--- a/src/cli.c
+++ b/src/cli.c
@@ -2770,7 +2770,7 @@ int pcli_wait_for_response(struct stream *s, struct channel *rep, int an_bit)
                s->csb->flags &= CS_FL_ISBACK | CS_FL_DONT_WAKE; /* we're in the context of process_stream */
                s->req.flags &= ~(CF_SHUTW|CF_SHUTW_NOW|CF_AUTO_CONNECT|CF_WRITE_ERROR|CF_STREAMER|CF_STREAMER_FAST|CF_NEVER_WAIT|CF_WROTE_DATA);
                s->res.flags &= ~(CF_SHUTR|CF_SHUTR_NOW|CF_READ_ATTACHED|CF_READ_ERROR|CF_READ_NOEXP|CF_STREAMER|CF_STREAMER_FAST|CF_WRITE_PARTIAL|CF_NEVER_WAIT|CF_WROTE_DATA|CF_READ_NULL);
-               s->flags &= ~(SF_DIRECT|SF_ASSIGNED|SF_ADDR_SET|SF_BE_ASSIGNED|SF_FORCE_PRST|SF_IGNORE_PRST);
+               s->flags &= ~(SF_DIRECT|SF_ASSIGNED|SF_BE_ASSIGNED|SF_FORCE_PRST|SF_IGNORE_PRST);
                s->flags &= ~(SF_CURR_SESS|SF_REDIRECTABLE|SF_SRV_REUSED);
                s->flags &= ~(SF_ERR_MASK|SF_FINST_MASK|SF_REDISP);
                s->conn_retries = 0;  /* used for logging too */

diff --git a/src/dns.c b/src/dns.c
index 29d3cef39881aed53168d6fb9654df124e99156e..e7e42ff1c4a611614b8cc544f8cddac7fb4b1f36 100644 (file)
--- a/src/dns.c
+++ b/src/dns.c
@@ -917,7 +917,7 @@ static struct appctx *dns_session_create(struct dns_session *ds)
        s->csb->dst = addr;
        s->csb->flags |= CS_FL_NOLINGER;
        s->target = &ds->dss->srv->obj_type;
-       s->flags = SF_ASSIGNED|SF_ADDR_SET;
+       s->flags = SF_ASSIGNED;
 
        s->do_log = NULL;
        s->uniq_id = 0;

diff --git a/src/hlua.c b/src/hlua.c
index 3dd2667102d09e4bcc28c31d5344fe90c20bf84e..06fa163dc023637f690b0e5ded334e849d149b58 100644 (file)
--- a/src/hlua.c
+++ b/src/hlua.c
@@ -2781,7 +2781,6 @@ __LJMP static int hlua_socket_connect(struct lua_State *L)
                xref_unlock(&socket->xref, peer);
                WILL_LJMP(luaL_error(L, "connect: internal error"));
        }
-       s->flags |= SF_ADDR_SET;
 
        /* Get hlua struct, or NULL if we execute from main lua state */
        hlua = hlua_gethlua(L);

diff --git a/src/http_client.c b/src/http_client.c
index e12d556e0ffe7d8864cc8ecb042a82723f86884a..0d34ca8c6dc6f3aa368c5ea43fecb8b5e36929e1 100644 (file)
--- a/src/http_client.c
+++ b/src/http_client.c
@@ -536,7 +536,7 @@ struct appctx *httpclient_start(struct httpclient *hc)
 
        s->csb->dst = addr;
        s->csb->flags |= CS_FL_NOLINGER;
-       s->flags |= SF_ASSIGNED|SF_ADDR_SET;
+       s->flags |= SF_ASSIGNED;
        s->res.flags |= CF_READ_DONTWAIT;
 
        /* applet is waiting for data */

diff --git a/src/peers.c b/src/peers.c
index 3c396958a39bbeca3f71d725d7c26dbec66c8e5f..522f2dbd820ffa3d36c098936266c37fb14dc3c8 100644 (file)
--- a/src/peers.c
+++ b/src/peers.c
@@ -3209,7 +3209,7 @@ static struct appctx *peer_session_create(struct peers *peers, struct peer *peer
        /* initiate an outgoing connection */
        s->csb->dst = addr;
        s->csb->flags |= CS_FL_NOLINGER;
-       s->flags = SF_ASSIGNED|SF_ADDR_SET;
+       s->flags = SF_ASSIGNED;
        s->target = peer_session_target(peer, s);
 
        s->do_log = NULL;

diff --git a/src/queue.c b/src/queue.c
index c2dd981d6b16e343f7317e61ddce684147d88772..94f9da8586d081edb85680ad9b94c7c56f370d10 100644 (file)
--- a/src/queue.c
+++ b/src/queue.c
@@ -495,7 +495,7 @@ int pendconn_redistribute(struct server *s)
 
                /* it's left to the dispatcher to choose a server */
                __pendconn_unlink_srv(p);
-               p->strm_flags &= ~(SF_DIRECT | SF_ASSIGNED | SF_ADDR_SET);
+               p->strm_flags &= ~(SF_DIRECT | SF_ASSIGNED);
 
                task_instant_wakeup(p->strm->task, TASK_WOKEN_RES);
                xferred++;
@@ -595,11 +595,11 @@ int pendconn_dequeue(struct stream *strm)
        /* the pendconn is not queued anymore and will not be so we're safe
         * to proceed.
         */
-       strm->flags &= ~(SF_DIRECT | SF_ASSIGNED | SF_ADDR_SET);
-       strm->flags |= p->strm_flags & (SF_DIRECT | SF_ASSIGNED | SF_ADDR_SET);
+       strm->flags &= ~(SF_DIRECT | SF_ASSIGNED);
+       strm->flags |= p->strm_flags & (SF_DIRECT | SF_ASSIGNED);
 
        /* the entry might have been redistributed to another server */
-       if (!(strm->flags & SF_ADDR_SET))
+       if (!(strm->flags & SF_ASSIGNED))
                sockaddr_free(&strm->csb->dst);
 
        if (p->target) {

diff --git a/src/sink.c b/src/sink.c
index b89ebb925c48c61ca9b4836bdc3c7ed7fa171166..1ef2bacbf7ae9f5a703f7e33f6d66ebebcd3f8ec 100644 (file)
--- a/src/sink.c
+++ b/src/sink.c
@@ -660,7 +660,7 @@ static struct appctx *sink_forward_session_create(struct sink *sink, struct sink
        s->csb->flags |= CS_FL_NOLINGER;
 
        s->target = &sft->srv->obj_type;
-       s->flags = SF_ASSIGNED|SF_ADDR_SET;
+       s->flags = SF_ASSIGNED;
 
        s->do_log = NULL;
        s->uniq_id = 0;