Take two. The change is that this version also tries to skip IPv4 addresses if the socket is IPv6-only. This case should have been handled by existing code paths, but the way that was done was quite twisted and apparently broke down by my changes to commResetFD.
This patch backs out part of the patch for Bug #2222 and replaces it by crudely cycling over the available addresses, trying to skip over addresses not compatible with the current socket. This solves issues seen when using tproxy or tcp_outgoing_address and DNS of the requested host returns AAAA records in addition to A records. This change is interim, waiting for the larger connection setup overhaul. But seems to do the trick at least for me in tproxy & ipv4 setups. Unfortunately I do not have an IPv6 connection at the moment to test IPv6 on, but I don't see how it could break IPv6. One effect of this change is that there will be no fallback to the other IP generation if the socket is configured to a specific outgoing address. Priory the code threw away the outgoing address and tried again when encountering an incompatibility. Regards Henrik
# Bazaar merge directive format 2 (Bazaar 0.90) # revision_id: [email protected]\ # e2go7bs5qme4867n # target_branch: http://squid-cache.org/bzr/squid3/branches/SQUID_3_1/ # testament_sha1: 57e227bd02cd6d333e95edd320cabe633e8cb02a # timestamp: 2010-05-13 13:33:11 +0200 # base_revision_id: [email protected]\ # 85q5fquskamzv6pu # # Begin patch === modified file 'src/comm.cc' --- src/comm.cc 2010-04-23 01:17:20 +0000 +++ src/comm.cc 2010-05-13 11:19:59 +0000 @@ -968,9 +968,6 @@ int ConnectStateData::commResetFD() { - struct addrinfo *AI = NULL; - IpAddress nul; - int new_family = AF_UNSPEC; // XXX: do we have to check this? // @@ -979,21 +976,19 @@ statCounter.syscalls.sock.sockets++; - /* setup a bare-bones addrinfo */ - /* TODO INET6: for WinXP we may need to check the local_addr type and setup the family properly. */ - nul.GetAddrInfo(AI); - new_family = AI->ai_family; + fde *F = &fd_table[fd]; + struct addrinfo *AI = NULL; + F->local_addr.GetAddrInfo(AI); int fd2 = socket(AI->ai_family, AI->ai_socktype, AI->ai_protocol); - nul.FreeAddrInfo(AI); - if (fd2 < 0) { debugs(5, DBG_CRITICAL, HERE << "WARNING: FD " << fd2 << " socket failed to allocate: " << xstrerror()); if (ENFILE == errno || EMFILE == errno) fdAdjustReserved(); + F->local_addr.FreeAddrInfo(AI); return 0; } @@ -1013,17 +1008,15 @@ close(fd2); + F->local_addr.FreeAddrInfo(AI); return 0; } commResetSelect(fd); close(fd2); - fde *F = &fd_table[fd]; - - /* INET6: copy the new sockets family type to the FDE table */ - fd_table[fd].sock_family = new_family; - - fd_table[fd].flags.called_connect = 0; + + F->flags.called_connect = 0; + /* * yuck, this has assumptions about comm_open() arguments for * the original socket @@ -1034,9 +1027,6 @@ comm_set_transparent(fd); } - AI = NULL; - F->local_addr.GetAddrInfo(AI); - if (commBind(fd, *AI) != COMM_OK) { debugs(5, DBG_CRITICAL, "WARNING: Reset of FD " << fd << " for " << F->local_addr << " failed to bind: " << xstrerror()); F->local_addr.FreeAddrInfo(AI); @@ -1104,8 +1094,7 @@ void ConnectStateData::connect() { - if (S.IsAnyAddr()) - defaults(); + defaults(); debugs(5,5, HERE << "to " << S); @@ -1122,15 +1111,22 @@ callCallback(COMM_OK, 0); break; -#if USE_IPV6 case COMM_ERR_PROTOCOL: + debugs(5, 5, HERE "FD " << fd << ": COMM_ERR_PROTOCOL - try again"); /* problem using the desired protocol over this socket. - * count the connection attempt, reset the socket, and immediately try again */ + * skip to the next address and hope it's more compatible + * but do not mark the current address as bad + */ tries++; - commResetFD(); - connect(); + if (commRetryConnect()) { + /* Force an addr cycle to move forward to the next possible address */ + ipcacheCycleAddr(host, NULL); + eventAdd("commReconnect", commReconnect, this, this->addrcount == 1 ? 0.05 : 0.0, 0); + } else { + debugs(5, 5, HERE << "FD " << fd << ": COMM_ERR_PROTOCOL - ERR tried too many times already."); + callCallback(COMM_ERR_CONNECT, errno); + } break; -#endif default: debugs(5, 5, HERE "FD " << fd << ": * - try again"); @@ -1232,18 +1228,25 @@ debugs(5, 9, "comm_connect_addr: connecting socket " << sock << " to " << address << " (want family: " << F->sock_family << ")"); - /* 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. + /* Handle IPv6 over IPv4-only socket case. * 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 + * 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 + + /* Handle IPv4 over IPv6-only socket case. + * This case is presently handled here as it's both a known case and it's + * uncertain what error will be returned by the IPv6 stack in such case. It's + * possible this will also be handled by the errno checks below after connect() + * but needs carefull cross-platform verification, and verifying the address + * condition here is simple. + */ + if (F->local_addr.IsIPv4() != address.IsIPv4()) { + return COMM_ERR_PROTOCOL; + } address.GetAddrInfo(AI, F->sock_family); @@ -1337,23 +1340,10 @@ status = COMM_OK; else if (ignoreErrno(errno)) status = COMM_INPROGRESS; + else if (errno == EAFNOSUPPORT || errno == EINVAL) + return COMM_ERR_PROTOCOL; else -#if USE_IPV6 - if ( address.IsIPv4() && F->sock_family == AF_INET6 ) { - - /* failover to trying IPv4-only link if an IPv6 one fails */ - /* to catch the edge case of apps listening on IPv4-localhost */ - F->sock_family = AF_INET; - int res = comm_connect_addr(sock, address); - - /* if that fails too, undo our temporary socktype hack so the repeat works properly. */ - if (res == COMM_ERROR) - F->sock_family = AF_INET6; - - return res; - } else -#endif - return COMM_ERROR; + return COMM_ERROR; address.NtoA(F->ipaddr, MAX_IPSTRLEN); # Begin bundle IyBCYXphYXIgcmV2aXNpb24gYnVuZGxlIHY0CiMKQlpoOTFBWSZTWct99BIABb9fgGZwcff////n 3wq////+YAo/eaaWt8NjbUtNFhlDRpTYqjRbVbOGUSnkmn6ptTbUTE2moYE9TTZJgE00MGoMjCeo DmE0BoDRowjQYjTEyYmgwjQMgGTA5hNAaA0aMI0GI0xMmJoMI0DIBkwEiIgAgkyeqPKekeKHpG0a gBoNGgDTag00EUhAmiYBNMqe01NJ4p6Cab0jSeUyGQADRo0EkSMRoI0noJgJEelPJPU8p6maDTT0 UGaIZPKaJIAkdmHJ9na/BdSuTLb/6o3e2FaKvhTidHb4Fx3X4NbFz0iZ02fyolX/9YefTdSjsjHa glDQZph1cTh6ayVF0vJpu5qHVqjIZ1NBN9hxbI30irLFeRk0lLLTDLMmeSADxkACVCXyLdnYYjy2 nXncPch8GITggTYA2jxX+Po1/US5NeHA7saZDUXV5W68rnmh4wYux0dl8A0+dmRNKWVbLGS5pM1O e8urZI7eFSWSpO6yjtJrLXBmfxreZr6tX2NvNGvaMQ1nqN+aY4ce6BnXStvoU/xjcY6Fr/z1Zndb dpSc4h4WbJMzjIveRpdTD+sZTvyymL/J66X3V61Q38FuU7XDi1w3Wqjwum6PTkUqe1TKolVXPxmm RLVYTsrPMwzAxVXarcbZBV+2hKgpYYDQLCUCc4Z/uHuZtb8jn4pi0wiVruWILSo4WjgMgwHoLFCj dGnSNbwwr89UPKZ+UAwVc4yXRVqKF4MJPcmmG56D4w9tXWLkfx4b33xnY/cIL9Ztu+ZfSeVpuIm1 8Y5NF3LPNY7B84nxCoSGEfSa2AvbfQljRDEnMPU7fIW3PgDxFpawRdg5QtOkcawI7F6n2es1D0dG 3JllKUpedXHVplFquaAaQZoDToZdVgYplUSblWFWLoB1FQybiKikrukcEmjAhVhMkVL0FCyK8xA8 hU6Gv+JgTJn2OVxudCuebIGQ5YNg86zURonBYJJlBDX5NQ/A6bBEJXoGioyAiEFkFWIkqmIg6U1P J0YbpCoVQstgpC9GAjZOmZvRKliksiHXEqi4nhmZy/fcV3Qh5hpVYmkMp/wmq4ylwmcuNrGOttNb soMVhlISUSZpRmOKBoHFcLQbzO5KDMUMHKdpZBtNYWlIMyYnUWRxGiKoIlpIvJE1iFMBrGk2whiV sUcuFgpRpiVvBmhNpFSGVJCwoN2pmBSBMRKfFSQozUncYG1Laam6mzLdxENMzDMwJAtETEOjyonD G0ayKqINAiJ8dQPGdxUiBerjeg6CjcasL2YcXFXyDPyNrW1MGOxQ7oRmzLadzMoOrYYYJEM+0wUJ o1Aolg3kvP3U13h3Cu7MtTbSx2ed+8LxxcSs7AsYyt7CpiJcUDtFmYGJs2QMTSvIYTCGbc7kchTB V2GpAzeXZKDQxMgqVIMOUv0AXkjvByBMmF4smnaah4thU49ID5DsJNVoHnJdFCAZXpkW9CRE2Dlx aaDmszsvoWxcxsathXoIjlB9O2hgrUFlCSTk5hQrYcLS2kq8UG7koz01wR3lxtDRkBOaMhKs9oeT 5f9kkgPKPiA8GKPtGjDlAgX2sLI7WPw72sbTlTysmSc7vhiALXkPURD8zjZtjIaWMmZ0DFTZ9Aj4 F9vo8z/mW4iA0sQQkNfvDBZAf7ThyUz3fSEczS/+5e9gvHJ3foRj0ub+be9hKMwzzOQhNzWS1Ffj dm+QwviLJ9cl72XgH/Ejb6ZuDPbsCaCst0p4g4EII/AB/FC2AXcrj5Ny+eeQmDqH5VVlwjeOh1zD b9fy1wQ2Y5SKJ7D6iSb/sqPUVneTO49RBkPdd7cnsMegdridoE1jjQrNQu9BspswD3tyaa3IivSo GFqcPObjqeH+abACqRBDxYX5+F+U8Z82s92gXuORvJHU9Zdp48eJb4DFYUM+C21WK4MVAPWklqIb xwf3F5gewgaiB7zd/UP7/XgLK6Auwx+HHtHdS12MhPFYfCYpWch5hqDzJ/W0fBdD16JnVh0qcxD2 Y7FEIM1jj93n1LlvKiCXAgRFWBxHVYlbYzEj+R7Ohg7AyZBG8vOpUe3me2HQyN6DqjJGgRrvEGWt CWzzK48kWPKNr9E2aoqOGuyKjjIrlyacr6OUiCU80t3WaA6wpUVaJnLqeBYnnHDhNxEORzRRrQUK uwquOpvHVhnL1GIp0bd3D9TRcYGFjpGDf4QYdOliXpWkEKXFji6BxJlrptdRNsonfRGfMGs1Dh1p CzQ4GDQgEPwiuladEINZjW1uWoMzxm458zSbzgbiKYsmTM1/348eqLDEX4nmJn15ozgBzQXp8Mfp VJlQBt7wt71VkobGZcEDchGZ4XUVM36F2naDomQDgZB6hCtOxB2efBc6Crr2glNmqWU3T1vGJ2Cz Kx38MOJKkFuHjLAL63YdOxDJd6ezP4WpucDewKXeLTUw/OgyceXxuO44TQ8z0PhK4DcMxGxm9KG1 CdBHDVgaAPvhwxnhoO8d1ZImDy1LLqHNCa3oOsNaDH1NjXr3vKGnUMhHnGQGlotOwL0xYzpeZkNy 40FyA0WJep95FsrxEZqXzTYqOIcVhwG5BeyTLAuBK77VkhSQQ1Jgr7RHSsiw9zBobULA1ic4jZFq o4YFtgWEMJevKYjWbpYieGGppuXtDOFrQTJSBWpQwgselzPNNM1COzm6Fm1SD+34K8Qt2kDg0UEo Fke7O/xW3rOzBZs0WN1rvd7vAqrhLUed4550M2AwwXt4w5/W8HYhAXvyCXuwTe5I3b5kkmkMkNjY SnVJFqiOQ5TiXaFeyADTcvwPLqNT42RxsCTDiKXDnJJCtpdoA3IxCYZsIVsgVRyeNEKbCthteNqG u96hlRmXu008FJgLupQWkewFJvUCb1Epjbi2xKUNfhhITOlZx4UGZazuAuk9bbwY2MwRuYDVYVEG 1CXbSdOPh63FxpXCLKVa3ZWZmigrocrJwDfIguCee3kh+7JVWxt1NAvaZNuyqCKYikCkmcgZIIQR 8iTIkSUa4OgpsBNXiKkXCKxcNdsIG5fwEaQxzWOlrKIqrqk2lneJ+KHW1L1jD3Z8rbfm0ITcRNBx iTz/FqSfu7RLEM4lLA13OuTpMJVxEPoGxkF3ZyWKpxZbKCBcKzGiCuj/F3JFOFCQy330Eg==
