On Fri, Mar 06, 2020 at 01:15:40PM +0100, Tim Duesterhus wrote:
> @@ -1466,6 +1466,20 @@ int make_proxy_line_v2(char *buf, int buf_len, struct
> server *srv, struct connec
> ret += make_tlv(&buf[ret], (buf_len - ret),
> PP2_TYPE_AUTHORITY, value_len, value);
> }
> }
> +
> + if (srv->pp_opts & SRV_PP_V2_UNIQUE_ID) {
> + struct session* sess = strm_sess(strm);
> + BUG_ON(sess != conn->owner); /* XXX is this correct? */
I'm pretty sure this one will break from time to time, maybe during
retries or when reusing an idle connection or something. Better drop
it. You don't really care who's the owner of the connection you're
using anyway. In case of multiplexing it could be anyone because
the connection will plugged onto by several streams before it's
finally connected and once it's OK and the sending code starts, you
have no way to tell whether the selected stream is the one that asked
for the connection first.
> + struct ist unique_id = stream_generate_unique_id(strm,
> &sess->fe->format_unique_id);
> + value = unique_id.ptr;
> + value_len = unique_id.len;
Also above, please do two things:
- always leave a blank line between declarations and statements, that
allow to quickly spot where variables are declared
- and as such please don't place BUG_ON() in the declarations since
it's code :-)
> + if (value_len >= 0) {
> + if ((buf_len - ret) < sizeof(struct tlv))
> + return 0;
> + ret += make_tlv(&buf[ret], (buf_len - ret),
> PP2_TYPE_UNIQUE_ID, value_len, value);
> + }
> + }
>
> #ifdef USE_OPENSSL
> if (srv->pp_opts & SRV_PP_V2_SSL) {
> diff --git a/src/log.c b/src/log.c
> index 8f502ac7e..2cac07486 100644
> --- a/src/log.c
> +++ b/src/log.c
> @@ -137,7 +137,7 @@ static const struct logformat_type logformat_keywords[] =
> {
> { "CC", LOG_FMT_CCLIENT, PR_MODE_HTTP, LW_REQHDR, NULL }, /* client
> cookie */
> { "CS", LOG_FMT_CSERVER, PR_MODE_HTTP, LW_RSPHDR, NULL }, /* server
> cookie */
> { "H", LOG_FMT_HOSTNAME, PR_MODE_TCP, LW_INIT, NULL }, /* Hostname */
> - { "ID", LOG_FMT_UNIQUEID, PR_MODE_HTTP, LW_BYTES, NULL }, /* Unique ID
> */
> + { "ID", LOG_FMT_UNIQUEID, PR_MODE_TCP, LW_BYTES, NULL }, /* Unique ID */
> { "ST", LOG_FMT_STATUS, PR_MODE_TCP, LW_RESP, NULL }, /* status code
> */
> { "T", LOG_FMT_DATEGMT, PR_MODE_TCP, LW_INIT, NULL }, /* date GMT */
> { "Ta", LOG_FMT_Ta, PR_MODE_HTTP, LW_BYTES, NULL }, /* Time active
> (tr to end) */
> diff --git a/src/server.c b/src/server.c
> index 965570222..908439d00 100644
> --- a/src/server.c
> +++ b/src/server.c
> @@ -656,6 +656,8 @@ static int srv_parse_proxy_v2_options(char **args, int
> *cur_arg,
> newsrv->pp_opts |= SRV_PP_V2_AUTHORITY;
> } else if (!strcmp(p, "crc32c")) {
> newsrv->pp_opts |= SRV_PP_V2_CRC32C;
> + } else if (!strcmp(p, "unique-id")) {
> + newsrv->pp_opts |= SRV_PP_V2_UNIQUE_ID;
> } else
> goto fail;
> }
> diff --git a/src/stream_interface.c b/src/stream_interface.c
> index 47cbd9693..d4acea8bc 100644
> --- a/src/stream_interface.c
> +++ b/src/stream_interface.c
> @@ -354,9 +354,11 @@ int conn_si_send_proxy(struct connection *conn, unsigned
> int flag)
> if (cs && cs->data_cb == &si_conn_cb) {
> struct stream_interface *si = cs->data;
> struct conn_stream *remote_cs =
> objt_cs(si_opposite(si)->end);
> + struct stream *strm = si_strm(si);
Same here, a blank line please, even though I know you were not the first
one to miss it.
> ret = make_proxy_line(trash.area, trash.size,
> objt_server(conn->target),
> - remote_cs ? remote_cs->conn :
> NULL);
> + remote_cs ? remote_cs->conn :
> NULL,
> + strm);
> /* We may not have a conn_stream yet, if we don't
> * know which mux to use, because it will be decided
> * during the SSL handshake. In this case, there should
(...)
So modulo the very few minor stuff above I'm overall OK without your
patch, just let me know if you have any question.
Thanks!
Willy
