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.sha...@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. _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev