On 02/14/2017 12:25 PM, Alex wrote: > It seems that the patch doesn't make things worse (if I understood you > correctly).
AFAICT, you are not testing the use case that prompted [trunk] revision 12742.1.41 changes. There are three possibilities: 1. r12742.1.41 changes were bogus/useless. 2. Your changes break use cases supported by r12742.1.41 changes. 3. Your changes do not break use cases supported by r12742.1.41 because all of them have a COMM_TRANSPARENT flag set. To know which outcome is correct, we need a test case that: A. Does not work without r12742.1.41 logic. B. Works with the current (i.e., post-r12742.1.41) unpatched code. C. Does not have a COMM_TRANSPARENT flag set. If such a test case exists, then #2 is correct. Unfortunately, I do not have a ready-to-use instructions on how to construct such a use case, but it probably has to involve: * direct/non-intercepted control connection (hence, no COMM_TRANSPARENT flag if you are testing patched Squid code); * multiple IP addresses from which Squid can open FTP data connections to the FTP client (i.e., the FTP client IP has to be reachable from the Squid box from two different source IP addresses); * TCP stack selecting the wrong default source IP address when establishing a connection from Squid to the FTP client. When all of the above bullets are satisfied, r12742.1.41 starts to matter in the patched code, meeting conditions A-C above. AFAICT, your current test cases do not satisfy the last two bullets. In other words, you patch removes setting the source IP address of the Squid data connection to the destination address of the control connection when there is no COMM_TRANSPARENT flag. Squid r12742.1.41 claims that such setting is necessary for some use cases to work. It is not clear whether r12742.1.41 is only relevant to COMM_TRANSPARENT connections. If it is, then your patch does not break things. If it is not, then your patch probably does. Hope this clarifies, Alex. _______________________________________________ squid-dev mailing list squid-dev@lists.squid-cache.org http://lists.squid-cache.org/listinfo/squid-dev