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.

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);
 

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 */

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))

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;
 		}

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 */

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;

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);

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 */

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;

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) {

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;