On Sat, Sep 14, 2019 at 12:40 AM Numan Siddique <nusid...@redhat.com> wrote:
>
>
>
> On Sat, Sep 14, 2019 at 2:41 AM Daniel Alvarez Sanchez <
dalva...@redhat.com> wrote:
>>
>> Acked-by: Daniel Alvarez <dalva...@redhat.com>
>>
>>
>> On Fri, Sep 13, 2019 at 11:02 PM Mark Michelson <mmich...@redhat.com>
wrote:
>> >
>> > Acked-by: Mark Michelson
>> >
>> > It sucks that we lose the efficiency of the conjunctive match
altogether
>> > on port groups because of this error, but I understand this is a huge
>> > bug and needs fixing.
>> If I'm not mistaken, from OpenStack standpoint conjunction was *only*
>> being used when using port groups and ACLs that matched on port ranges
>> ( e.g tcp.dst >= X && tcp.dst <=Y) which was not working. Therefore
>> we're not losing performance because it was already broken (given that
>> there was more than one ACL like that).
>>
>> >
>> > Perhaps it would be good to start up a discussion on this list about a
>> > more longterm solution that would allow for conjunctive matches with no
>> > ambiguity.
>> Agreed! We already discussed some ideas on IRC but it'd be awesome to
>> have a thread and brainstorm there.
>>
>
> Thanks for the reviews. I applied this to master.
> Agree we can discuss it further and come up with ideas.
>
> I know Dumitru has some idea to make use of conjunctions for port groups.
> CC'ing Han if he has any comments on ideas.
>
Hi Numan,
This is a good finding. However, I think it is not specifically a problem
of port group. It seems to be a more general problem and this patch fixes
only a special case.
For example, would there be similar problem for below ACLs without port
groups:
ip4 && tcp.src >= 1000 && tcp.src <= 1001 && tcp.dst >= 500 && tcp.dst <=
501
ip4 && tcp.src >= 1000 && tcp.src <= 1001 && tcp.dst >= 600 && tcp.dst <=
601
Another example is with address set:
ip4 && ip4.src == $as1 && tcp.dst >= 500 && tcp.dst <= 501
ip4 && ip4.src == $as1 && tcp.dst >= 600 && tcp.dst <= 601
Or even without range:
ip4 && tcp.src == {1000, 1001} && tcp.dst == {500, 501}
ip4 && tcp.src == {1000, 1001} && tcp.dst == {600, 601}
You may think of more examples. Whenever there are multiple conjunctionable
ACLs with same match as part of the conjunction, it should result in such
problem.
A quick fix to all these problems may be just abandon conjunction, but I
believe there are better ways to address it.
First of all, these matches can be rewritten by combining them in a single
ACL with "OR" operator, e.g.:
outport == @pg1 && ip4 && tcp.dst >= 500 && tcp.dst <= 501
outport == @pg1 && ip4 && tcp.dst >= 600 && tcp.dst <= 601
can be rewritten as ====>
outport == @pg1 && ip4 && (tcp.dst >= 500 && tcp.dst <= 501 || tcp.dst >=
600 && tcp.dst <= 601)
Similar can be done for all above examples. So, a workaround to the problem
is from user side (e.g. OpenStack) to make sure always combining ACLs with
"OR" if there are common conjunctionable matches between different ACLs.
However, a better way would be in ovn-northd itself to detect and combine
such ACLs internally, before generating the final logical flows in SB. It
may be more convenient to be done in ovn-controller, because we are not
even parsing the ACLs in ovn-northd today, but the cost of such
pre-processing would be duplicated in all HVs. It surely will increase CPU
cost for doing such combination, but I'd not worry too much if we do it
properly at each LS level instead of for all ACLs.
Thanks,
Han
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
