On Fri, Jan 11, 2019 at 12:16:39AM +0000, Ankur Sharma wrote: > This patch allows user to associate a value with acl, > which will be assigned to ct.label of the corresponding > connection tracking entry. > > This value can be used to map a ct entry with corresponding > OVN ACL or higher level constructs like security group. > > signed-off-by: Ankur Sharma <ankur.sha...@nutanix.com>
Thanks for the patch! Please capitalize the "S" in "Signed-off-by". This adds a column in ovn-sb.ovsschema, so it should increment the minor version (the y in x.y.z). The documentation for the new column explains what it does, but it does not explain the purpose. Why would a user set this column? What are its effects? The column is a string, but its value is an integer. Maybe that is because OVSDB integer columns are limited to 64 bits, but this value can be 128 bits. That is a very large space. What is the reason that so much space should be dedicated to this identifier? Even 64 bits is more identifiers than any deployment will ever use, so there must be some other reason. Please do not use // comments. Please document the new option in the ovn-sbctl manpage. Please add a NEWS item for the new feature. _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
