<div>The problem is that <span style="color:rgb(0,0,0);font-family:Arial,sans-serif;font-size:15px;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:normal;text-align:start;text-transform:none;white-space:pre-wrap;background-color:rgb(255,255,255);display:inline !important;float:none;">r12742.1.41 is bogus and useless at the same time due to the following reasons:</span></div><div><span style="color:rgb(0,0,0);font-family:Arial,sans-serif;font-size:15px;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:normal;text-align:start;text-transform:none;white-space:pre-wrap;background-color:rgb(255,255,255);display:inline !important;float:none;">1. To bind some non-local IP address one needs COMM_TRANSPARENT flag which will trigger IP_TRANSPARENT socket option. The flag was not set, therefore attempts to bind any non-local address are bound to fail.</span></div><div><span style="font-size:15px;">2. For COMM_TRANSPARENT we need CAP_NET_ADMIN, but this capability is set only when </span><span style="color:rgb(0,0,0);font-family:Arial,sans-serif;font-size:15px;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:normal;text-align:start;text-transform:none;white-space:pre-wrap;background-color:rgb(255,255,255);display:inline !important;float:none;">'ftp_port' option is set to 'tproxy'</span></div><div> </div><div><span style="color:rgb(0,0,0);font-family:Arial,sans-serif;font-size:15px;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:normal;text-align:start;text-transform:none;white-space:pre-wrap;background-color:rgb(255,255,255);display:inline !important;float:none;">Taking this into account it may be concluded that the patch doesn't break this [tproxy] logic in any way.</span></div><div><br /></div><div><br /></div><div>14.02.2017, 23:23, "Alex Rousskov" <rousskov@measurement-factory.com>:</div><blockquote type="cite"><p>On 02/14/2017 12:25 PM, Alex wrote:<br /></p><blockquote> It seems that the patch doesn't make things worse (if I understood you<br /> correctly).<br /></blockquote><p><br />AFAICT, you are not testing the use case that prompted [trunk] revision<br />12742.1.41 changes. There are three possibilities:<br /><br />  1. r12742.1.41 changes were bogus/useless.<br />  2. Your changes break use cases supported by r12742.1.41 changes.<br />  3. Your changes do not break use cases supported by r12742.1.41<br />     because all of them have a COMM_TRANSPARENT flag set.<br /><br />To know which outcome is correct, we need a test case that:<br /><br />  A. Does not work without r12742.1.41 logic.<br />  B. Works with the current (i.e., post-r12742.1.41) unpatched code.<br />  C. Does not have a COMM_TRANSPARENT flag set.<br /><br />If such a test case exists, then #2 is correct. Unfortunately, I do not<br />have a ready-to-use instructions on how to construct such a use case,<br />but it probably has to involve:<br /><br />* direct/non-intercepted control connection (hence, no COMM_TRANSPARENT<br />flag if you are testing patched Squid code);<br /><br />* multiple IP addresses from which Squid can open FTP data connections<br />to the FTP client (i.e., the FTP client IP has to be reachable from the<br />Squid box from two different source IP addresses);<br /><br />* TCP stack selecting the wrong default source IP address when<br />establishing a connection from Squid to the FTP client.<br /><br />When all of the above bullets are satisfied, r12742.1.41 starts to<br />matter in the patched code, meeting conditions A-C above. AFAICT, your<br />current test cases do not satisfy the last two bullets.<br /><br /><br />In other words, you patch removes setting the source IP address of the<br />Squid data connection to the destination address of the control<br />connection when there is no COMM_TRANSPARENT flag. Squid r12742.1.41<br />claims that such setting is necessary for some use cases to work. It is<br />not clear whether r12742.1.41 is only relevant to COMM_TRANSPARENT<br />connections. If it is, then your patch does not break things. If it is<br />not, then your patch probably does.<br /><br /><br />Hope this clarifies,<br /><br />Alex.<br /><br /></p></blockquote>