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 8052e8d..3a2b48a 100644
--- a/dev/flags/flags.c
+++ b/dev/flags/flags.c
@@ -402,7 +402,6 @@
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 5e3541f..4a017f4 100644
--- 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 b77c993..5660857 100644
--- a/include/haproxy/stream.h
+++ b/include/haproxy/stream.h
@@ -339,7 +339,7 @@
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 c584453..3b0e231 100644
--- a/src/backend.c
+++ b/src/backend.c
@@ -1349,13 +1349,9 @@
* 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 @@
*/
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 b32b8b1..7f6b58b 100644
--- a/src/cli.c
+++ b/src/cli.c
@@ -2770,7 +2770,7 @@
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 29d3cef..e7e42ff 100644
--- a/src/dns.c
+++ b/src/dns.c
@@ -917,7 +917,7 @@
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 3dd2667..06fa163 100644
--- a/src/hlua.c
+++ b/src/hlua.c
@@ -2781,7 +2781,6 @@
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 e12d556..0d34ca8 100644
--- a/src/http_client.c
+++ b/src/http_client.c
@@ -536,7 +536,7 @@
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 3c39695..522f2db 100644
--- a/src/peers.c
+++ b/src/peers.c
@@ -3209,7 +3209,7 @@
/* 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 c2dd981..94f9da8 100644
--- a/src/queue.c
+++ b/src/queue.c
@@ -495,7 +495,7 @@
/* 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 @@
/* 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 b89ebb9..1ef2bac 100644
--- a/src/sink.c
+++ b/src/sink.c
@@ -660,7 +660,7 @@
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;