Re: patch: nested acl evaluation
On Sat, Apr 4, 2009 at 3:30 PM, Willy Tarreau wrote: > On Sat, Apr 04, 2009 at 10:20:23AM +0800, Jeffrey 'jf' Lim wrote: >> > OK maybe "use" is OK in fact, considering the alternatives. >> > >> >> :) some proposals for the keywords: >> >> for/use >> condition/use >> cond/use >> >> (cond/use seems the best compromise - short, but understandable enough) > > what would you think about "do/use" ? > I actually prefer cond/use (more intuitive with the idea of an "if" block), but hey, you're the man... :P >> > If we extend the system, we'll have to associate parameters >> > with a condition. >> > >> > But since the entry point is the switching rule here, maybe we'll end up >> > with something very close to what you have implemented in your patch, in >> > the end, and it's just that we want to make it more generic to use the >> > two conditions layers in any ruleset. >> > >> >> I would guess so! Even the redirect rules and 'block' rules look >> pretty similar... :) > > yes, and maybe this is what we should try to improve : converge towards > a generic condition matching/execution system, to which we pass the > action and the args. That way, we just have to run it over redirect_rules > or switching_rules always using the same code. > :) right... :D Does this sound like fun? lol -jf
Re: patch: nested acl evaluation
On Sat, Apr 04, 2009 at 10:20:23AM +0800, Jeffrey 'jf' Lim wrote: > > OK maybe "use" is OK in fact, considering the alternatives. > > > > :) some proposals for the keywords: > > for/use > condition/use > cond/use > > (cond/use seems the best compromise - short, but understandable enough) what would you think about "do/use" ? > > If we extend the system, we'll have to associate parameters > > with a condition. > > > > But since the entry point is the switching rule here, maybe we'll end up > > with something very close to what you have implemented in your patch, in > > the end, and it's just that we want to make it more generic to use the > > two conditions layers in any ruleset. > > > > I would guess so! Even the redirect rules and 'block' rules look > pretty similar... :) yes, and maybe this is what we should try to improve : converge towards a generic condition matching/execution system, to which we pass the action and the args. That way, we just have to run it over redirect_rules or switching_rules always using the same code. Willy
Re: patch: nested acl evaluation
On Fri, Apr 3, 2009 at 3:11 PM, Willy Tarreau wrote: > On Fri, Apr 03, 2009 at 01:37:53PM +0800, Jeffrey 'jf' Lim wrote: >> > OK. Just so that I get an idea, how many use_backend rules (and how many >> > backends) do you have in a large config ? I'm asking because I want to be >> > able to support ACL files and rules files, and the way to implement them >> > should mostly be driven by large config users. >> > >> >> we're on the way to reaching 100 use_backend rules even as we speak... >> :) (and because these rules are largely of the pattern i've >> mentioned.. this motivated this patch here) > > OK, that's still pretty reasonable. I know configurations with 500 "listen" > sections, and fortunately they don't use ACLs ! > right. >> > I have no problem with "use" if we only support "use_backend", but >> > I was thinking about extending it to support other keywords, and >> > thought it would be a bit awkward with keywords such as "redirect" >> > or "block". Maybe I'm wrong and "use" fits all usages ? >> > >> >> 'use' is kind of generic, i suppose. I have to admit, though - that >> was proposed with my usage situation and tons of 'use_backend' >> swimming around in my head! :P But I get your point - you want to >> expand this to other keywords. > > OK maybe "use" is OK in fact, considering the alternatives. > :) some proposals for the keywords: for/use condition/use cond/use (cond/use seems the best compromise - short, but understandable enough) >> Yeah, I had to cut it in two halves cos of the "overloading" of the >> 'use_backend' keyword... Of course if we use a separate keyword, then >> nothing like that happens :P I cut acl processing in half too - but of >> course, i guess that's only a given since we want to be able to reduce >> processing anyway. > > Well, keep in mind that processing ACLs is rather cheap anyway. Since most > of the CPU time is already spent in system, even if you would multiply > haproxy's work by 10, you would only cut performance in half. I have > memories of something like less than 10-20% performance loss with 1000 ACLs, > but I may be wrong. That does not prevent us from optimising for massive > usage though ;-) > half is good for me! :) > (...) >> > While we would have something more like this : >> > >> > use_backend_block if X or Y >> > use B1 if X1 or Y1 or Z1 >> > use B2 if X2 or Y2 or Z2 >> > >> > => foreach acl in X Y; do >> > if (acl) >> > foreach rule in B1 B2; do >> > foreach acl in X1 Y1 Z1; do >> > if (acl) use_backend $B >> > >> >> hm... I dunno. I think it's more of like this? (this is great for >> dealing with 'and' conditions as well, btw) >> >> if X or Y; do // or could be 'X and Y'! 'unless X', ... whatever >> if X1 or Y1 or Z1 >> use_backend B1 >> else if X2 or Y2 or Z2 >> use_backend B2 > > Yes, you're right, in fact your "if" above are the call to acl_exec_cond() > while my "foreach acl in" was also the call to acl_exec_cond(). I just wanted > to emphasize on the fact that we're not scanning ACLs then deciding on an > action, but rather check all conditions for an actions and perform it if a > rule matches. right. > If we extend the system, we'll have to associate parameters > with a condition. > > But since the entry point is the switching rule here, maybe we'll end up > with something very close to what you have implemented in your patch, in > the end, and it's just that we want to make it more generic to use the > two conditions layers in any ruleset. > I would guess so! Even the redirect rules and 'block' rules look pretty similar... :) -jf
Re: patch: nested acl evaluation
On Fri, Apr 03, 2009 at 01:37:53PM +0800, Jeffrey 'jf' Lim wrote: > > OK. Just so that I get an idea, how many use_backend rules (and how many > > backends) do you have in a large config ? I'm asking because I want to be > > able to support ACL files and rules files, and the way to implement them > > should mostly be driven by large config users. > > > > we're on the way to reaching 100 use_backend rules even as we speak... > :) (and because these rules are largely of the pattern i've > mentioned.. this motivated this patch here) OK, that's still pretty reasonable. I know configurations with 500 "listen" sections, and fortunately they don't use ACLs ! > > I have no problem with "use" if we only support "use_backend", but > > I was thinking about extending it to support other keywords, and > > thought it would be a bit awkward with keywords such as "redirect" > > or "block". Maybe I'm wrong and "use" fits all usages ? > > > > 'use' is kind of generic, i suppose. I have to admit, though - that > was proposed with my usage situation and tons of 'use_backend' > swimming around in my head! :P But I get your point - you want to > expand this to other keywords. OK maybe "use" is OK in fact, considering the alternatives. > Yeah, I had to cut it in two halves cos of the "overloading" of the > 'use_backend' keyword... Of course if we use a separate keyword, then > nothing like that happens :P I cut acl processing in half too - but of > course, i guess that's only a given since we want to be able to reduce > processing anyway. Well, keep in mind that processing ACLs is rather cheap anyway. Since most of the CPU time is already spent in system, even if you would multiply haproxy's work by 10, you would only cut performance in half. I have memories of something like less than 10-20% performance loss with 1000 ACLs, but I may be wrong. That does not prevent us from optimising for massive usage though ;-) (...) > > While we would have something more like this : > > > > use_backend_block if X or Y > > use B1 if X1 or Y1 or Z1 > > use B2 if X2 or Y2 or Z2 > > > > => foreach acl in X Y; do > > if (acl) > > foreach rule in B1 B2; do > > foreach acl in X1 Y1 Z1; do > > if (acl) use_backend $B > > > > hm... I dunno. I think it's more of like this? (this is great for > dealing with 'and' conditions as well, btw) > > if X or Y; do // or could be 'X and Y'! 'unless X', ... whatever > if X1 or Y1 or Z1 > use_backend B1 > else if X2 or Y2 or Z2 > use_backend B2 Yes, you're right, in fact your "if" above are the call to acl_exec_cond() while my "foreach acl in" was also the call to acl_exec_cond(). I just wanted to emphasize on the fact that we're not scanning ACLs then deciding on an action, but rather check all conditions for an actions and perform it if a rule matches. If we extend the system, we'll have to associate parameters with a condition. But since the entry point is the switching rule here, maybe we'll end up with something very close to what you have implemented in your patch, in the end, and it's just that we want to make it more generic to use the two conditions layers in any ruleset. Regards, Willy
Re: patch: nested acl evaluation
On Fri, Apr 3, 2009 at 1:00 PM, Willy Tarreau wrote:
> On Fri, Apr 03, 2009 at 08:55:11AM +0800, Jeffrey 'jf' Lim wrote:
> (...)
>>
>> Having said that, there are blocks already in the standard config
>> (like "backend", with multiple "server" lines) - but the difference is
>> they don't need an ending keyword, so ok I see the problem here is
>> with the mandatory end (guess that's what you mean by "section", vs
>> "block"!)
>
> yes. It don't like it when we need to explicitly terminate a block (or
> section if you want). This always leads to trouble in the long term,
> because of missing or inverted termination statements. So the language
> must be unambiguous enough not to require this. BTW, that's what you
> have in cisco-like configs.
>
:) now that you mention it, yeah...
>> > With all that in mind, I'm wondering if what you want is not just sort
>> > of a massive remapping feature. I see that you have arbitrarily factored
>> > on the Host part and you are selecting the backend based on the path
>> > part. Is this always what you do ? Are there any other cases ?
>>
>> for us atm, no. (Although i do appreciate the flexibility, of course,
>> to factor on anything else ['for_acl' will do this right now]. Key
>> thing for me is, I want to reduce acl processing when we've got big
>> sticking common acl among all the statements [you've seen my
>> examples])
>
> OK. Just so that I get an idea, how many use_backend rules (and how many
> backends) do you have in a large config ? I'm asking because I want to be
> able to support ACL files and rules files, and the way to implement them
> should mostly be driven by large config users.
>
we're on the way to reaching 100 use_backend rules even as we speak...
:) (and because these rules are largely of the pattern i've
mentioned.. this motivated this patch here)
>
>> > If you think you still need ACLs but
>> > just for use_backend rules, maybe we should just use a slightly
>> > different keyword : simply not repeat "use_backend" and use "select"
>> > instead, which does not appear in the normal config section :
>> >
>> > use_backend bk1 if acl1
>> > use_backend_block if Host1
>> > select bk1 if path1 or path2
>> > select bk2 if path3
>> > select bk3 if path4 src1
>> > ...
>> > use_backend bkN if aclN
>> >
>> > That one would present the advantage of being more intuitive and
>> > would integrate better with other rules. Also, it would make it
>> > more intuitive how to write such blocks for other rule sets, and
>> > is very close to what you've already done. And that does not
>> > require any end tag since the keyword used in the block ("select"
>> > above) is not present in the containing block.
>> >
>>
>> right! I would be fine with this. Perhaps we could use the keyword
>> "use" instead of "select"? so
>>
>> use_backend_block if host_www
>> use bk1 if path 1 or path 2
>> use bk2 if path3
>> use bk3 if path4 src1
>> ...
>
> I have no problem with "use" if we only support "use_backend", but
> I was thinking about extending it to support other keywords, and
> thought it would be a bit awkward with keywords such as "redirect"
> or "block". Maybe I'm wrong and "use" fits all usages ?
>
'use' is kind of generic, i suppose. I have to admit, though - that
was proposed with my usage situation and tons of 'use_backend'
swimming around in my head! :P But I get your point - you want to
expand this to other keywords.
(oh, and I don't like call/with either).
>> I can trivially modify my patch to be able to do this.
>
> The problem is not to modify a patch but more to do it the right
> way. Right now, from what I've seen, your patch cuts the use_backend
> processing in two halves, while I don't think we should implement it
> that way.
>
Yeah, I had to cut it in two halves cos of the "overloading" of the
'use_backend' keyword... Of course if we use a separate keyword, then
nothing like that happens :P I cut acl processing in half too - but of
course, i guess that's only a given since we want to be able to reduce
processing anyway.
>> > Maybe with a little bit more thinking we could come up with something
>> > more generic like this :
>> >
>> > ...
>> > call use_backend if acl1
>> > with bk1 if path1 or path2
>> > with bk2 if ...
>> > ...
>> >
>> > call redirect if acl1
>> > with prefix http://here.local if path2
>> > with location / if path3
>> > ...
>> >
>> > See ? That would basically add an iterator around any type of ACL
>> > rule, providing us with the ability to only specify the verb on
>> > the first line, and all the args in the list, and this would make
>> > a lot of sense. I don't like the "call" nor "with" keywords, it's
>> > just an illustration.
>> >
>>
>> ok. Adding this would be interesting, although wow, lots of code... :P
>
> Not necessarily. Right now, your code wraps around use_backend only.
> Instead, we could extend the rules themselves to be two-dimensional.
> Instead of evalu
Re: patch: nested acl evaluation
On Fri, Apr 03, 2009 at 08:55:11AM +0800, Jeffrey 'jf' Lim wrote:
(...)
> > This is the first statement which introduces the notion
> > of a block, with a beginning and a mandatory end. I don't like that
> > for the same reason I don't like config languages with braces. It's
> > harder to debug and maintain.
> >
>
> I dunno. I'm finding the "line by line" thing harder to debug! (I once
> made an error where the hostname was wrong because I copied the config
> en masse from another hostname - and somehow missed out on changing
> the hostname for this one line).
>
> Having said that, there are blocks already in the standard config
> (like "backend", with multiple "server" lines) - but the difference is
> they don't need an ending keyword, so ok I see the problem here is
> with the mandatory end (guess that's what you mean by "section", vs
> "block"!)
yes. It don't like it when we need to explicitly terminate a block (or
section if you want). This always leads to trouble in the long term,
because of missing or inverted termination statements. So the language
must be unambiguous enough not to require this. BTW, that's what you
have in cisco-like configs.
> > With all that in mind, I'm wondering if what you want is not just sort
> > of a massive remapping feature. I see that you have arbitrarily factored
> > on the Host part and you are selecting the backend based on the path
> > part. Is this always what you do ? Are there any other cases ?
>
> for us atm, no. (Although i do appreciate the flexibility, of course,
> to factor on anything else ['for_acl' will do this right now]. Key
> thing for me is, I want to reduce acl processing when we've got big
> sticking common acl among all the statements [you've seen my
> examples])
OK. Just so that I get an idea, how many use_backend rules (and how many
backends) do you have in a large config ? I'm asking because I want to be
able to support ACL files and rules files, and the way to implement them
should mostly be driven by large config users.
(...)
> > See ? No more ACLs on the path and direct mapping between path and
> > backends for a given host.
>
> yeah. Except that I kind of think ACLs are more flexible. That, and I
> remember reading your note in the docs that regexes are slower than
> other ACLs. (I primarily use 'path_beg').
OK.
> Also, using ACL names would
> be more readable - and have the bonus (this is probably very important
> for me) of providing a form of error checking as well (you check that
> an acl actually exists in the code).
Good point!
> > If you think you still need ACLs but
> > just for use_backend rules, maybe we should just use a slightly
> > different keyword : simply not repeat "use_backend" and use "select"
> > instead, which does not appear in the normal config section :
> >
> > use_backend bk1 if acl1
> > use_backend_block if Host1
> > select bk1 if path1 or path2
> > select bk2 if path3
> > select bk3 if path4 src1
> > ...
> > use_backend bkN if aclN
> >
> > That one would present the advantage of being more intuitive and
> > would integrate better with other rules. Also, it would make it
> > more intuitive how to write such blocks for other rule sets, and
> > is very close to what you've already done. And that does not
> > require any end tag since the keyword used in the block ("select"
> > above) is not present in the containing block.
> >
>
> right! I would be fine with this. Perhaps we could use the keyword
> "use" instead of "select"? so
>
> use_backend_block if host_www
> use bk1 if path 1 or path 2
> use bk2 if path3
> use bk3 if path4 src1
> ...
I have no problem with "use" if we only support "use_backend", but
I was thinking about extending it to support other keywords, and
thought it would be a bit awkward with keywords such as "redirect"
or "block". Maybe I'm wrong and "use" fits all usages ?
> I can trivially modify my patch to be able to do this.
The problem is not to modify a patch but more to do it the right
way. Right now, from what I've seen, your patch cuts the use_backend
processing in two halves, while I don't think we should implement it
that way.
> > Maybe with a little bit more thinking we could come up with something
> > more generic like this :
> >
> > ...
> > call use_backend if acl1
> > with bk1 if path1 or path2
> > with bk2 if ...
> > ...
> >
> > call redirect if acl1
> > with prefix http://here.local if path2
> > with location / if path3
> > ...
> >
> > See ? That would basically add an iterator around any type of ACL
> > rule, providing us with the ability to only specify the verb on
> > the first line, and all the args in the list, and this would make
> > a lot of sense. I don't like the "call" nor "with" keywords, it's
> > just an illustration.
> >
>
> ok. Adding this would be interesting, although wow, lots of code... :P
Not necessarily. Right now, your code wraps around use_backend only.
Instead, we
Re: patch: nested acl evaluation
On Fri, Apr 3, 2009 at 5:20 AM, Willy Tarreau wrote:
> Hi Jeffrey,
>
> On Thu, Apr 02, 2009 at 02:23:44PM +0800, Jeffrey 'jf' Lim wrote:
> (...)
>> Ok perhaps "combinatorial" was not the word that i should have used,
>> but... I hope you can see the point/s with the explanation that i
>> gave. The "head" acl only gets checked once - thereafter which it goes
>> into the body (you could treat it like the standard "if" statement in
>> any programming language) to do the evaluation.
>>
>> Without this, the "head" acl has to be evaluated every time for every
>> combination of "head" acl + "sub" acl for you can go into each
>> 'use_backend'. So eg:
>>
>> use_backend b1 if host_www.domain.com path1
>> use_backend b2 if host_www.domain.com path2
>> use_backend b3 if host_www.domain.com path3
>> use_backend b4 if host_www.domain path4 or host_www.domain path5
>> ...
>>
>> use_backend b10 if host_www.d2.com path1
>> use_backend b11 if host_www.d2.com path2
>> ...
>>
>>
>> So does it make sense to cut out all of this? Of course it does.
>> Faster acl processing (you dont repeat having to process
>> 'host_www.domain.com' every time!), much neater (and maintainable)
>> config file.
>>
>> ==
>>
>> The following is a refined patch (did i mention i was serious about
>> this? just to allay the "April Fool's" thing)
>>
>> One caveat to take note: at this point in time I'm not going to cater
>> for nested 'for_acl's (I think if u really need this, you probably
>> have bigger problems). One level of 'for_acl' should be all you
>> need... (why is this deja vu) (Willy, I'll work out the documentation
>> later once/if u give the go ahead for this, thanks)
>
> I understand the usefulness of your proposal, but I really dislike
> the implementation for at least two reasons :
> - right now the config works on a line basis. There are sections, but
> no block.
yeah, I realize :P
> This is the first statement which introduces the notion
> of a block, with a beginning and a mandatory end. I don't like that
> for the same reason I don't like config languages with braces. It's
> harder to debug and maintain.
>
I dunno. I'm finding the "line by line" thing harder to debug! (I once
made an error where the hostname was wrong because I copied the config
en masse from another hostname - and somehow missed out on changing
the hostname for this one line).
Having said that, there are blocks already in the standard config
(like "backend", with multiple "server" lines) - but the difference is
they don't need an ending keyword, so ok I see the problem here is
with the mandatory end (guess that's what you mean by "section", vs
"block"!)
> - only the "use_backend" rules are covered. So the "for_acl" keyword
> is not suited since in fact you're just processing a "use_backend"
> list.
right!
> Also, that leaves me with two new questions : what could be
> done for other rules ? Do we need to duplicate all the code, or can
> we factor the same block out and use it everywhere ? Second
> question : maybe you need this for use_backend only, which marks it
> as a special case which might be handled even more easily ?
>
yes, I want it really just for 'use_backend' only. Keyword wise, I was
struggling to find something useful. I ended up with 'for_acl' :P But
I get what u mean. My bad - I was focussing only on 'use_backend' cos
that's what we have a lot of here. :P
> With all that in mind, I'm wondering if what you want is not just sort
> of a massive remapping feature. I see that you have arbitrarily factored
> on the Host part and you are selecting the backend based on the path
> part. Is this always what you do ? Are there any other cases ?
for us atm, no. (Although i do appreciate the flexibility, of course,
to factor on anything else ['for_acl' will do this right now]. Key
thing for me is, I want to reduce acl processing when we've got big
sticking common acl among all the statements [you've seen my
examples])
> You still
> have to declare ACLs for each path. Maybe it would be better to simply
> support something like this :
>
> use_backend back1 if acl1
> map_path_to_backend if Host1
> use BK1 on ^/img
> use BK2 on ^/js
> use BK3 on ^/static/.*\.{jpg|gif|png}
> ...
> use_backend backN if aclN
>
>
> See ? No more ACLs on the path and direct mapping between path and
> backends for a given host.
yeah. Except that I kind of think ACLs are more flexible. That, and I
remember reading your note in the docs that regexes are slower than
other ACLs. (I primarily use 'path_beg'). Also, using ACL names would
be more readable - and have the bonus (this is probably very important
for me) of providing a form of error checking as well (you check that
an acl actually exists in the code).
> If you think you still need ACLs but
> just for use_backend rules, maybe we should just use a slightly
> different keyword : simply not repeat "use_backend" and use "select"
> instead, which does no
Re: patch: nested acl evaluation
Hi Jeffrey,
On Thu, Apr 02, 2009 at 02:23:44PM +0800, Jeffrey 'jf' Lim wrote:
(...)
> Ok perhaps "combinatorial" was not the word that i should have used,
> but... I hope you can see the point/s with the explanation that i
> gave. The "head" acl only gets checked once - thereafter which it goes
> into the body (you could treat it like the standard "if" statement in
> any programming language) to do the evaluation.
>
> Without this, the "head" acl has to be evaluated every time for every
> combination of "head" acl + "sub" acl for you can go into each
> 'use_backend'. So eg:
>
> use_backend b1 if host_www.domain.com path1
> use_backend b2 if host_www.domain.com path2
> use_backend b3 if host_www.domain.com path3
> use_backend b4 if host_www.domain path4 or host_www.domain path5
> ...
>
> use_backend b10 if host_www.d2.com path1
> use_backend b11 if host_www.d2.com path2
> ...
>
>
> So does it make sense to cut out all of this? Of course it does.
> Faster acl processing (you dont repeat having to process
> 'host_www.domain.com' every time!), much neater (and maintainable)
> config file.
>
> ==
>
> The following is a refined patch (did i mention i was serious about
> this? just to allay the "April Fool's" thing)
>
> One caveat to take note: at this point in time I'm not going to cater
> for nested 'for_acl's (I think if u really need this, you probably
> have bigger problems). One level of 'for_acl' should be all you
> need... (why is this deja vu) (Willy, I'll work out the documentation
> later once/if u give the go ahead for this, thanks)
I understand the usefulness of your proposal, but I really dislike
the implementation for at least two reasons :
- right now the config works on a line basis. There are sections, but
no block. This is the first statement which introduces the notion
of a block, with a beginning and a mandatory end. I don't like that
for the same reason I don't like config languages with braces. It's
harder to debug and maintain.
- only the "use_backend" rules are covered. So the "for_acl" keyword
is not suited since in fact you're just processing a "use_backend"
list. Also, that leaves me with two new questions : what could be
done for other rules ? Do we need to duplicate all the code, or can
we factor the same block out and use it everywhere ? Second
question : maybe you need this for use_backend only, which marks it
as a special case which might be handled even more easily ?
With all that in mind, I'm wondering if what you want is not just sort
of a massive remapping feature. I see that you have arbitrarily factored
on the Host part and you are selecting the backend based on the path
part. Is this always what you do ? Are there any other cases ? You still
have to declare ACLs for each path. Maybe it would be better to simply
support something like this :
use_backend back1 if acl1
map_path_to_backend if Host1
use BK1 on ^/img
use BK2 on ^/js
use BK3 on ^/static/.*\.{jpg|gif|png}
...
use_backend backN if aclN
See ? No more ACLs on the path and direct mapping between path and
backends for a given host. If you think you still need ACLs but
just for use_backend rules, maybe we should just use a slightly
different keyword : simply not repeat "use_backend" and use "select"
instead, which does not appear in the normal config section :
use_backend bk1 if acl1
use_backend_block if Host1
select bk1 if path1 or path2
select bk2 if path3
select bk3 if path4 src1
...
use_backend bkN if aclN
That one would present the advantage of being more intuitive and
would integrate better with other rules. Also, it would make it
more intuitive how to write such blocks for other rule sets, and
is very close to what you've already done. And that does not
require any end tag since the keyword used in the block ("select"
above) is not present in the containing block.
Maybe with a little bit more thinking we could come up with something
more generic like this :
...
call use_backend if acl1
with bk1 if path1 or path2
with bk2 if ...
...
call redirect if acl1
with prefix http://here.local if path2
with location / if path3
...
See ? That would basically add an iterator around any type of ACL
rule, providing us with the ability to only specify the verb on
the first line, and all the args in the list, and this would make
a lot of sense. I don't like the "call" nor "with" keywords, it's
just an illustration.
I'd like to get opinions for other massive ACL users, because any
minor change might have significant impact to many of us, and we
must keep in mind that what we develop has to be maintained over
the years, sometimes conflicting with further features.
Best regards,
Willy
Re: patch: nested acl evaluation
On Wed, Apr 1, 2009 at 6:05 PM, Jeffrey 'jf' Lim wrote:
> Combinatorial acls, everybody!!! :) The usefulness of this patch is
> best explained by a config snippet, so here goes...
>
> ===
> for_acl if host_www.domain.com
>
> use_backend b1 if path1
>
> use_backend b2 if path2
> use_backend b3 if path3
>
> use_backend b4 if path4 or path5
>
> use_backend b5 if path6 or path7 or path8
> use_backend b6 if path9
>
> endfor_acl
> ===
>
> (Yeah, I know i had to sanitize some names ;) but I hope u get the
> usefulness of this! Saner config for me, faster acl processing as
> well. Can be used in a hosting situation as well, where multiple
> hostnames are tied to the same ip)
>
>
(Some extra commentary):
Ok perhaps "combinatorial" was not the word that i should have used,
but... I hope you can see the point/s with the explanation that i
gave. The "head" acl only gets checked once - thereafter which it goes
into the body (you could treat it like the standard "if" statement in
any programming language) to do the evaluation.
Without this, the "head" acl has to be evaluated every time for every
combination of "head" acl + "sub" acl for you can go into each
'use_backend'. So eg:
use_backend b1 if host_www.domain.com path1
use_backend b2 if host_www.domain.com path2
use_backend b3 if host_www.domain.com path3
use_backend b4 if host_www.domain path4 or host_www.domain path5
...
use_backend b10 if host_www.d2.com path1
use_backend b11 if host_www.d2.com path2
...
So does it make sense to cut out all of this? Of course it does.
Faster acl processing (you dont repeat having to process
'host_www.domain.com' every time!), much neater (and maintainable)
config file.
==
The following is a refined patch (did i mention i was serious about
this? just to allay the "April Fool's" thing)
One caveat to take note: at this point in time I'm not going to cater
for nested 'for_acl's (I think if u really need this, you probably
have bigger problems). One level of 'for_acl' should be all you
need... (why is this deja vu) (Willy, I'll work out the documentation
later once/if u give the go ahead for this, thanks)
diff -ur haproxy-1.3.17-PRISTINE/include/types/proxy.h
haproxy-1.3.17/include/types/proxy.h
--- haproxy-1.3.17-PRISTINE/include/types/proxy.h 2009-03-29
21:26:57.0 +0800
+++ haproxy-1.3.17/include/types/proxy.h2009-04-01 17:53:23.714670412
+0800
@@ -277,8 +277,14 @@
struct error_snapshot invalid_req, invalid_rep; /* captures of last
errors */
};
+/* values for switching_rule->type */
+#define SWR_SIMPLE 0
+#define SWR_NESTED 1
+
struct switching_rule {
+ short type; /* jf: acl type - simple... or
a complex 'if' body */
struct list list; /* list linked to from the
proxy */
+ struct list second_rules; /* jf: 2nd-level switching
rulesz */
struct acl_cond *cond; /* acl condition to meet */
union {
struct proxy *backend; /* target backend */
diff -ur haproxy-1.3.17-PRISTINE/src/cfgparse.c haproxy-1.3.17/src/cfgparse.c
--- haproxy-1.3.17-PRISTINE/src/cfgparse.c 2009-03-29 21:26:57.0
+0800
+++ haproxy-1.3.17/src/cfgparse.c 2009-04-02 13:48:03.604287568 +0800
@@ -131,6 +131,7 @@
int cfg_maxpconn = DEFAULT_MAXCONN;/* # of simultaneous connections
per proxy (-N) */
int cfg_maxconn = 0; /* # of simultaneous connections, (-n)
*/
unsigned int acl_seen = 0; /* CFG_ACL_* */
+short in_for_acl = 0;
/* List head of all known configuration keywords */
static struct cfg_kw_list cfg_keywords = {
@@ -1407,12 +1408,87 @@
}
rule = (struct switching_rule *)calloc(1, sizeof(*rule));
+ rule->type = SWR_SIMPLE;// !!jf: cos 'use_backend' IS
simple
rule->cond = cond;
rule->be.name = strdup(args[1]);
LIST_INIT(&rule->list);
- LIST_ADDQ(&curproxy->switching_rules, &rule->list);
+ if (in_for_acl)
+ LIST_ADDQ(&LIST_ELEM(curproxy->switching_rules.p, struct
switching_rule *, list)->second_rules, &rule->list);
+ else
+ LIST_ADDQ(&curproxy->switching_rules, &rule->list);
+
acl_seen |= CFG_ACL_BACKEND;
}
+ else if (!strcmp(args[0], "for_acl")) {
+
+ if (in_for_acl) {
+ Alert("parsing [%s:%d] : '%s' cannot be nested.\n",
file, linenum, args[0]);
+ return -1;
+ }
+
+ int pol = ACL_COND_NONE;
+ struct acl_cond *cond;
+ struct switching_rule *toprule;
+
+ if (curproxy == &
