[ovs-dev] [RFC PATCH v1 1/3] OVN ACL: Replace the usage of ct_label with ct_mark

Ben Pfaff blp at ovn.org
Tue Feb 5 21:21:15 UTC 2019


On Fri, Jan 11, 2019 at 12:16:35AM +0000, Ankur Sharma wrote:
> OVN ACL implementation used ct_label to indicate if a previosuly
> allowed connection shoudl not be allowed anymore and vice versa.
> 
> However, ct_label is a 128 bit value and we should rather leverage
> on ct_mark which is a 32 bit value.
> 
> Using ct_mark for this purpose, allows us to use ct_label for storing
> other values like, identifier for corresponidng OVN ACL/Security group etc.
> 
> signed-off-by: Ankur Sharma <ankur.sharma at nutanix.com>

Thanks for the patch.

I think that the idea here is to retain the existing ct_label.blocked
for compatibility with older ovn-northd, so that during an upgrade the
older logical flows continue to work.  That is a good idea.  I think
that there should be a comment in logical-fields.c explaining why
ct_label.blocked is still there.  Then, someday in the future, when we
think that upgrades from such an old version is no longer important, we
will have an idea why it is there and that we can now to remove it.

I find myself wondering, though, why we have ct_label.blocked at all.
In some other cases where ovn-northd uses a bit specifically, it has a
macro for it, e.g. REGBIT_CONNTRACK_COMMIT.  Another option would be to
have better abstraction, i.e. to name the bit "ct.blocked" instead of
ct_mark.blocked or ct_label.blocked.

Thanks,

Ben.


More information about the dev mailing list