[ovs-dev] [PATCH] ovn: Add second ACL stage

Russell Bryant russell at ovn.org
Tue Aug 2 20:14:36 UTC 2016


On Tue, Aug 2, 2016 at 3:35 PM, Guru Shetty <guru at ovn.org> wrote:

>
>
> On 2 August 2016 at 12:27, Russell Bryant <russell at ovn.org> wrote:
>
>>
>>
>> On Tue, Aug 2, 2016 at 3:17 PM, Guru Shetty <guru at ovn.org> wrote:
>>
>>>
>>>
>>> On 2 August 2016 at 12:01, Russell Bryant <russell at ovn.org> wrote:
>>>
>>>>
>>>> On Tue, Aug 2, 2016 at 1:29 PM, Guru Shetty <guru at ovn.org> wrote:
>>>>
>>>>> The 2 ct_commit for deletion of firewall rules will likely be tricky.
>>>>> This
>>>>> will need unit tests.
>>>>>
>>>>
>>>> I don't think I understand the concern.  Can you expand a bit on what
>>>> you mean by "2 ct_commit for deletion of firewall rules"?
>>>>
>>>
>>> My memory on how ct_commit(ct_label=1) works is a little hazy. There are
>>> 2 stages now. So whenever a firewall rule is deleted for an established
>>> connection, the default ct_commit(ct_label=1) will get hit and the
>>> connection is dropped. The same thing happens in the second stage for any
>>> removed firewall rule. In the second stage when a firewall rule is deleted
>>> ct_label is also set which will reflect in the first stage. Does not this
>>> cause confusion with the logic?
>>>
>>
>> Setting ct_label back to 0 only happens in the stateful table.  That
>> ct_commit will only occur if none of the ACL stages think the packet should
>> be dropped.  I think it's OK.
>>
>
> I see. I think we should still consider unit tests now. Userspace datapath
> has ct_commit now (it still can't do NAT). That should ideally work. If
> that does not work, we should consider adding tests to system-ovn.at
>

Yes, I agree that this area is sorely lacking in test coverage.

-- 
Russell Bryant



More information about the dev mailing list