On Tue, Mar 29, 2016 at 11:24 PM, Willy Tarreau <[email protected]> wrote:
> Hi Frederik,
>
> On Mon, Mar 28, 2016 at 02:31:27PM -0700, Frederik Deweerdt wrote:
>> Hi,
>>
>> I've been working on an issue we've been seeing on very high loads with
>> a configuration that boils down to:
>>
>> [SSL Traffic] ---> [HAProxy] ---[via send_proxy]--> [Proxy]
>>
>> We're seeing this with 1.5.12 but the latest git behaves in the same way
>> (Patches are against lastest git).
>>
>> We have custom code in conn_si_send_proxy() that looks at
>> remote->t.sock.fd, and does a getsockname() on it. Sometimes that call
>> fails with EBADFD.
>>
>> It appears that the reason this is happening is that frontend socket has
>> been closed via fd_delete() before we go and try to open a socket to the
>> backend (note that we could have checked conn->flags and see that the
>> CO_FL_ERROR flag was set).
> (...)
>> It seems that we would be a bit more efficient if we also aborted when
>> si_b->state was SI_ST_INI: that is, don't even try to open a connection
>> to the backend if we're shutting down the frontend.
>
> No, we should not do this. You can already force this behaviour with
> "option abortonclose".
Mmm, adding "option abortonclose" does work in "mode http", but not in
"mode tcp", which I've been using. I can however confirm that if the
check becomes:
(s->be->options & PR_O_ABRT_CLOSE || channel_is_empty(req))
rather than
channel_is_empty(req)
it does close the connection there.
> But we must not do this by default because we'd
> break a number of automated scripts basically doing :
>
> echo -e "GET /report?host=$HOST&load=$LOAD HTTP/1.0\r\n\r\n" | nc
> some-host 1234
>
Right, understood.
> We may have an issue with the way the address is retrieved from the
> connection. The source address should be correct since it's retrieved
> by accept(). It's for the destination address that we need the FD.
>
> I'm thinking about two possibilities :
> - either we consider that if we can't retrieve a connection's address
> for a proxy protocol line we must fail and abort the connection ;
> - or we consider that when we're closing a front connection early
> (with "early" still to be defined, maybe with something in the
> request buffer), then we retrieve the destination address prior
> to closing. Or maybe we should retrieve this each time the client
> closes first (read 0 or error caught) except when the session is
> idle.
>
> I guess we should do both. First ensure that we always have the
> socket's addresses before closing on error, and then cover the
> possible remaining cases by aborting outgoing connections with
> an incomplete proxy proto.
>
> What do you think ?
I hadn't considered this - I was trying to address the "don't open
a connection if the peer fd is closed" case - but I think that
both fixes sound good, with a preference for retrieving each
time the client closes first, since that has simpler semantics.
At the very least, it sounds like the proxy code should be looking
the CO_FL_ADDR_*_SET flags.
Regards,
Frederik
