On 7/10/26 11:05 PM, Dumitru Ceara wrote:
> Hi Ilya,
> 
> On 7/10/26 6:01 PM, Ilya Maximets wrote:
>> On 7/10/26 12:57 AM, Mairtin O'Loingsigh via dev wrote:
>>> Add a new "pass-related" action that behaves like "pass" for
>>> verdict (no verdict bit is set, so evaluation continues to
>>> the next tier) but follows the stateful path like
>>> "allow-related": it matches on hint registers, commits the
>>> connection via conntrack, and allows established/related
>>> traffic flows.
>>
>> I didn't read the code, so I'm not sure if ti is implemented this way,
>> but it feels like the semantics as it is described is not what it
>> supposed to be.  If I see pass-related, I think that the related
>> traffic will have pass semantics as well, i.e., will be passed through
>> this ACL tier and be evaluated in the next tier.  So, it will not just
>> be automatically allowed.  Does that make sense?
>>
> 
> I'm not sure we ever reached agreement on that.  But please see below on
> why I think the approach Mairtin is following is what we should pursue.
> 
>> Also, if the lower tier ACL allows the packet then what is the actual
>> difference between the higher tier ACL being pass or pass-related?
>>
>> This feature needs a lot more documentation for how it works.
>>
>> IIRC, the original description in the ticket is not really correct
>> and the way ACLs actually work wouldn't allow for what ovn-kubenetes
>> wanted.
>>
>> I vaguely remember that we ended up with the thought that pass-related
>> only makes sense for the same direction related traffic like FTP.
>> But I don't remember details.  Dumitru, do you remember more context?
>>
> 
> The way I remember it is that the current implementation has the right
> semantics for let's call them "older" uses of "multi-tier" pass ACLs in
> ovn-kubernetes.
> 
> That's because there's always a highest-tier allow-related ACL
> configured by ovn-kubernetes.
> 
> So traffic initiating new connections that hits "pass" rules in lower
> tiers and finally is either explicitly or implicitly allowed in the
> highest tier will always be committed to conntrack.
> 
> Replies (and related packets) for such traffic will find the existing
> conntrack entry and will be correctly allowed.
> 
> The only remaining problem is the case when we really have no
> allow-related ACLs configured on the switch.
> 
> And I think you're right the only hard to tackle case then is related
> traffic.
> 
> In a sandbox:
> 
>> ovn-nbctl show ls
> switch 31b890ac-f8ba-4c20-9e21-ebbd606e781e (ls)
>     port lsp1
>     port lsp2
> 
> # Explicitly drop FTP data traffic (port 20) in tier 1,
> # except for sessions allowed in tier 0.

This is a misleading description, as tier 0 doesn't allow anything.

>> ovn-nbctl acl-list ls
> from-lport   100 (inport == "lsp1" && tcp.dest == 21) pass [tier 0]
> from-lport   100 (tcp.dst == 20) drop [tier 1]
> 
> FTP data traffic from lsp1 (TCP dest port 20) will never make it through
> no matter what we add in tier 1, right?

You can make the tier 0 ACL allow-related to make it go through.

> 
> But with the "pass-related" semantics Mairtin is adding, we'd have:
> 
>> ovn-nbctl acl-list ls
> from-lport   100 (inport == "lsp1" && tcp.dest == 21) pass-related [tier 0]
> from-lport   100 (tcp.dst == 20) drop [tier 1]
> 
> The first FTP control packet (TCP SYN towards port 21) hits the ACL in
> tier 0.  It's a pass-related ACL so eventually, at the end of the
> complete ACL evaluation stage (after all tiers) we commit the session to
> conntrack.

But it is pass-related, not allow-related.  So, related connection should
be passed to the next tier and match there and be dropped by the tier 1 ACL.

The fact that you have pass-related turns every allow into allow-related
because everything is now committed into conntrack.  In this case the
implicit default-allow is turned into implicit default allow-related.

But you can achieve exactly the same behavior by using allow-related in the
tier 0 in the first place.  Or by adding a low priority explicit default
allow-related at the last tier, which pass-related in this implementation
appears to be just a misleading syntactic sugar for.

Does that make sense?

Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to