Re: Squid-3.1, tproxy & commResetFD

On Tue, 11 May 2010 01:20:38 +0200, Henrik Nordström
<henrik_at_henriknordstrom.net> wrote:
> Been looking at TPROXY support in 3.1, and good news is that it seems to
> mostly work.
>
> Bad news is that it interacts badly with hosts having IPv6 addresses in
> DNS, resulting in errors in cache.log + probably attempt to IPv6
> connection. Similar breakage probably also exists when
> tcp_outgoing_address is set to an ipv4 address.
>
>
> The culpit is here
>
>
> ConnectStateData::connect()
>
> #if USE_IPV6
> case COMM_ERR_PROTOCOL:
> /* problem using the desired protocol over this socket.
> * count the connection attempt, reset the socket, and
immediately
> try again */
> tries++;
> commResetFD();
> connect();
> break;
> #endif
>
> which comes from here:
>
> comm_connect_addr(int sock, const IpAddress &address)
>
> /* BUG 2222 FIX: reset the FD when its found to be IPv4 in IPv6 mode
*/
> /* inverse case of IPv4 failing to connect on IPv6 socket is handeld
> post-connect.
> * this case must presently be handled here since the GetAddrInfo
> asserts on bad mappings.
> * eventually we want it to throw a Must() that gets handled there
> instead of this if.
> * NP: because commresetFD is private to ConnStateData we have to
> return an error and
> * trust its handled properly.
> */
> #if USE_IPV6
> if (F->sock_family == AF_INET && !address.IsIPv4()) {
> return COMM_ERR_PROTOCOL;
> }
> #endif
>
> and because of this commResetFD resets the FD to IPv6, which isn't
> desired when using tproxy or tcp_outgoing_address, But fixing
> commResetFD to account for this breaks the above instead (infinite
> loop).
>
> When the outgoing address is set to IPv4 then we should not be looking
> for IPv6 addresses, or try to connect to IPv6 destinations.
>
> Not sure how to best fix this without rewriting comm_connect socket
> management to get rid of commResetFD.. (a change which is long overdue
> btw).
>
> Regards
> Henrik

This was one of the main factors behind my ConnectionDetails proposal for
forwarding. Which is awaiting implementation.

Amos
Received on Mon May 10 2010 - 23:27:41 MDT