On Thu, May 23, 2024 at 02:47:15PM +0100, William Manley wrote:
> On Thu, May 23, 2024, at 2:08 PM, Amaury Denoyelle wrote:
> > On Thu, May 23, 2024 at 11:55:13AM +0100, William Manley wrote:
> > > On Thu, May 23, 2024, at 11:34 AM, William Manley wrote:
> > > > On Thu, May 23, 2024, at 10:08 AM, Amaury Denoyelle wrote:
> > > > > On Wed, May 22, 2024 at 04:58:44PM +0100, William Manley wrote:
> > > > > > On Wed, May 22, 2024, at 1:06 PM, Amaury Denoyelle wrote:
> > > > > > > FYI, I just merged a series of fix to improve reverse HTTP. It is
> > > > > > > now
> > > > > > > possible to use PROXY protocol on preconnect stage. Also, you
> > > > > > > have the
> > > > > > > availability to use PROXY v2 TLV to differentiate connections.
> > > > > > > Note
> > > > > > > however that PROXY unique-id is not supported for now due to
> > > > > > > internal
> > > > > > > API limitations.
> > > > > > > > If you can do not hesitate to test this and report us if it's
> > > > > > > > sufficient
> > > > > > > for you.
> > > > > > I've just tried this out and there's something about these changes
> > > > > > that are causing my tests to fail.
> > > > > > It seems to be triggered by "MEDIUM: rhttp: create session for
> > > > > > active preconnect"
> > > > > > Tested versions:
> > > > > > eb89a7da33ae30da3ed61570aa1597987b59dff3 OK
> > > > > > ceebb09744df367ad84586a341d9336f84f72bce OK
> > > > > > 45b80aed70a597614e31b748328570785099dfec OK
> > > > > > 12c40c25a9520fe3365950184fe724a1f4e91d03 BAD
> > > > > > 60496e884e5220b9330a1d8b3a1218f7988c879a BAD
> > > > > > 4a968d9d274a24e5d00bd3c03dd22fe2563b13af BAD
> > > > > > I'll investigate further tomorrow.
> > > > > > > Hum can you describe what sort of tests you are running ?
> > > >
> > > > [...]
> > > >
> > > > I've attached the configs used during the test. You'll notice that
> > > > each block uses a different IP address from 127.0.0.*. This makes the
> > > > TCP flow graph in wireshark meaningful. I've attached a packet capture
> > > > of the test failing. I'd also attach a packet capture from it
> > > > succeeding, but it's a bit big. I'll look into them and try to figure
> > > > out where it's going wrong.
> > > I've compared the good trace and the bad trace and I think I can see
> > > where this is going wrong. My RHTTP block on the node looks like:
> > > # Requests from the portal to the node arrive at this frontend over rhttp:
> > > listen flask_reverse_proxy
> > > mode http
> > > log global
> > > bind rhttp@rhttp_backend/srv
> > > # We dynamically route to the listening socket based on the hostname, this
> > > # way we can add new domains without a node deploy:
> > > http-request capture req.hdr(X-Remote-IP) len 15
> > > http-request capture req.hdr(X-Remote-Port) len 5
> > > http-request set-dst hdr(X-Remote-IP)
> > > http-request set-dst-port hdr(X-Remote-Port)
> > > http-response add-header Vary X-Remote-IP,X-Remote-Port
> > > server srv 0.0.0.0:0 source 127.0.0.8
> > > It seems like with the active preconnect commit (12c40c25) it ignores
> > > set-dst, so instead of connecting to the IP requested (in this case
> > > 127.0.0.7) it attempts to connect to 127.0.0.8.
> > > Interesting. Could you please try the attached patch and tell me if this
> > solves your issue ? Many thanks,
> It doesn't fix it. It fails in the same way.
Thank you for your quick testing. I have manage to reproduce what I
think could be a similar issue. I have updated my patch, can you please
try the following new one ?
--
Amaury Denoyelle
>From 28785a6594915b7cf91a29a5240d052dbc2523eb Mon Sep 17 00:00:00 2001
From: Amaury Denoyelle <adenoye...@haproxy.com>
Date: Thu, 23 May 2024 15:06:52 +0200
Subject: [PATCH] BUG/MINOR: connection: fix session init on reverse
---
src/connection.c | 3 +++
src/proto_rhttp.c | 2 +-
2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/src/connection.c b/src/connection.c
index d186c2f01..cdb1c7aaa 100644
--- a/src/connection.c
+++ b/src/connection.c
@@ -2785,6 +2785,7 @@ int conn_reverse(struct connection *conn)
session_unown_conn(sess, conn);
sess->origin = NULL;
session_free(sess);
+ sess = NULL;
conn_set_owner(conn, NULL, NULL);
conn->flags |= CO_FL_REVERSED;
@@ -2802,6 +2803,8 @@ int conn_reverse(struct connection *conn)
/* Invert source and destination addresses if already set. */
SWAP(conn->src, conn->dst);
+ if (sess)
+ SWAP(sess->src, sess->dst);
conn->reverse.target = NULL;
ha_free(&conn->reverse.name.area);
diff --git a/src/proto_rhttp.c b/src/proto_rhttp.c
index 9bf02157c..25122f514 100644
--- a/src/proto_rhttp.c
+++ b/src/proto_rhttp.c
@@ -62,7 +62,7 @@ static struct connection *new_reverse_conn(struct listener
*l, struct server *sr
HA_ATOMIC_INC(&th_ctx->nb_rhttp_conns);
- sess = session_new(l->bind_conf->frontend, l, NULL);
+ sess = session_new(l->bind_conf->frontend, l, &conn->obj_type);
if (!sess)
goto err;
--
2.45.1
