Re: [PATCH] ACL to control TPROXY spoofing

On 27/02/2013 7:26 a.m., Alex Rousskov wrote:
> On 02/26/2013 05:17 AM, Steve Hill wrote:
>>> Code simplicity. An "if(flags.spoof)" test is far faster than even
>>> constructing a checklist and processing "allow all" in fast-ACL pathway.
>>> So if the ACL flexibility does not actually have a clear need the speed
>>> would be better.
>
>> Ok. Well I'm a bit on the fence here too.
>>
>> I can see some use for the flexibility - the situation I mentioned would
>> require spoofing to be disabled for requests from the branch offices but
>> it would probably be desirable to leave spoofing on for the main office.
> ...
>> I tend to think that since the ACL isn't constructed and tested in the
>> default case (and therefore for most people there is no performance
>> hit), I would err towards increased functionality rather than increased
>> performance.
> It sounds like Steve has a reasonable use case where ACLs would help.
> And he is right that the default should be "no acl" (with appropriate
> effect) rather than "allow all" ACL so that the feature performance
> impact on Squid that does not care about these things will be negligible
> and equivalent to the "if (flags.spoof)" test overheads.
>
> If you need a tie breaker, and there is no expert to chime in, I am
> happy to vote for the ACL control path, with a "no ACL" default :-).

I'm happy with the use-case too now. If the code adjusted to 3.HEAD
still looks good I think it could go in.

Amos
Received on Tue Feb 26 2013 - 21:56:29 MST