Re: [PATCH] ACL to control TPROXY spoofing

May I chime in?
While I'm not an expert too, why can't we have the best of both worlds
without even letting the cache admin know?
This could be as simple as having some code which detects if the
config has changed from the "deny all" default and enters the ACL
checking code path only if it is so.. there'd be a negligible overhead
in the uncommon case, and just a bit of unused code carried around in
the common case..

On Tue, Feb 26, 2013 at 7:26 PM, Alex Rousskov
<rousskov_at_measurement-factory.com> 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 :-).
>
>
> Thank you,
>
> Alex.
>

--
    /kinkie