)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":4694,"name":"Miguel Lavalle","email":"miguel@mlavalle.com","username":"minsel"},"change_message_id":"6fc03d7306803028d3007c867b4d1a84a8bb1738","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":15,"id":"694783da_d8c0ca46","updated":"2025-08-29 22:56:23.000000000","message":"-1 to draw your attention. The fix to the logical problem seems correct to me. However, in some edge cases, there might be a performance issue. Please see my in line comment. If you think it is a very extreme possibility, I\u0027ll be glad to +2 the patch.","commit_id":"079b0af98e031b77a5dc723cc8d39fdfbba3a6d6"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"b118413e6058fa162ef7abadd6877fb6455c2e88","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":20,"id":"f3db9fff_2f15c951","updated":"2025-09-08 07:41:31.000000000","message":"qq: shouldn\u0027t we prevent when inserting any ACL? When this could happen?","commit_id":"1c3eac52b724dd5f095ede0d17db432796c41516"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"42e0e23073ee37cf475cfe7dc36978893e88f116","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":20,"id":"7aa1325d_384d66f9","updated":"2025-11-17 17:06:30.000000000","message":"recheck unrelated functional failure","commit_id":"1c3eac52b724dd5f095ede0d17db432796c41516"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"f2f3423e147d4176cd106721c3fddb4e946d607d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":20,"id":"db25ec05_4b8310ed","in_reply_to":"f3db9fff_2f15c951","updated":"2025-09-08 14:31:51.000000000","message":"So the OVN DB will only ever have a single ACL, but there could be multiple Neutron SG rules that map to the same one. The issue is that only one of the SG rule IDs is used in the OVN ACL entry, so the second SG rule triggers a false positive warning saying there is still a rule out of sync.\n\nThe functional test is basically what I did locally to reproduce:\n\n        # Add SG rules that map to the same ACL due to normalizing the cidr\n        for ip_suffix in range(10, 12):\n            remote_ip_prefix \u003d \u0027192.168.0.\u0027 + str(ip_suffix) + \u0027/24\u0027\n            self._create_security_group_rule(\n                sg[\u0027id\u0027], \u0027ingress\u0027, 9000, remote_ip_prefix\u003dremote_ip_prefix)\n\nBasically add two rules with the same subnet cidr but different IP addresses.\n\nI\u0027m not sure we should change the API to throw a conflict on the second create since it could break users.","commit_id":"1c3eac52b724dd5f095ede0d17db432796c41516"}],"neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_db_sync.py":[{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"246c40a43a327a9d843e8eea87d33ab164345449","unresolved":true,"context_lines":[{"line_number":322,"context_line":"        # SG rule ID key. This eliminates any false-positives where the"},{"line_number":323,"context_line":"        # normalized cidr for two SG rules is the same value, since there"},{"line_number":324,"context_line":"        # will only be a single ACL that matches exactly with the SG rule ID."},{"line_number":325,"context_line":"        add_rem_acls \u003d []"},{"line_number":326,"context_line":"        for add_acl in add_acls:"},{"line_number":327,"context_line":"            add_acl_copy \u003d add_acl.copy()"},{"line_number":328,"context_line":"            del add_acl_copy[ovn_const.OVN_SG_RULE_EXT_ID_KEY]"}],"source_content_type":"text/x-python","patch_set":1,"id":"65d4691c_96953adc","line":325,"updated":"2024-12-06 23:48:08.000000000","message":"I do have to work on optimizing the loops below, even doing it on the opposite order - OVN first, found second, would be faster.","commit_id":"2fd08b60332865acc7975bc597f71aac0b53c58f"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"c093681d743db33c1db8d8127cea1d11ad6ca97f","unresolved":false,"context_lines":[{"line_number":322,"context_line":"        # SG rule ID key. This eliminates any false-positives where the"},{"line_number":323,"context_line":"        # normalized cidr for two SG rules is the same value, since there"},{"line_number":324,"context_line":"        # will only be a single ACL that matches exactly with the SG rule ID."},{"line_number":325,"context_line":"        add_rem_acls \u003d []"},{"line_number":326,"context_line":"        for add_acl in add_acls:"},{"line_number":327,"context_line":"            add_acl_copy \u003d add_acl.copy()"},{"line_number":328,"context_line":"            del add_acl_copy[ovn_const.OVN_SG_RULE_EXT_ID_KEY]"}],"source_content_type":"text/x-python","patch_set":1,"id":"a25d15f2_dec21b2e","line":325,"in_reply_to":"65d4691c_96953adc","updated":"2025-04-27 22:17:03.000000000","message":"Done","commit_id":"2fd08b60332865acc7975bc597f71aac0b53c58f"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"77c58f7026a21df769a2a98247b4ac53846e3f8e","unresolved":true,"context_lines":[{"line_number":329,"context_line":"                return acl_copy"},{"line_number":330,"context_line":""},{"line_number":331,"context_line":"            add_rem_acls \u003d []"},{"line_number":332,"context_line":"            # Make a list of non-default rule ACLs, as they do not have a"},{"line_number":333,"context_line":"            # security group rule id."},{"line_number":334,"context_line":"            # See ovn_default_acls code/comment above for more information."},{"line_number":335,"context_line":"            nd_ovn_acls \u003d [rem_id_key_copy(oa) for oa in ovn_acls"}],"source_content_type":"text/x-python","patch_set":5,"id":"4f2d4116_3158457f","line":332,"range":{"start_line":332,"start_character":60,"end_line":332,"end_character":66},"updated":"2025-04-27 22:43:08.000000000","message":"d/do not\n\nleftover from previous PS","commit_id":"0d99330c1785ced04c351cd559e722ef2f38482a"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"a40fd2dde20f016309dd09343ff722f25fe8ab05","unresolved":false,"context_lines":[{"line_number":329,"context_line":"                return acl_copy"},{"line_number":330,"context_line":""},{"line_number":331,"context_line":"            add_rem_acls \u003d []"},{"line_number":332,"context_line":"            # Make a list of non-default rule ACLs, as they do not have a"},{"line_number":333,"context_line":"            # security group rule id."},{"line_number":334,"context_line":"            # See ovn_default_acls code/comment above for more information."},{"line_number":335,"context_line":"            nd_ovn_acls \u003d [rem_id_key_copy(oa) for oa in ovn_acls"}],"source_content_type":"text/x-python","patch_set":5,"id":"93660ee4_a63a13c0","line":332,"range":{"start_line":332,"start_character":60,"end_line":332,"end_character":66},"in_reply_to":"4f2d4116_3158457f","updated":"2025-04-28 00:35:39.000000000","message":"Done","commit_id":"0d99330c1785ced04c351cd559e722ef2f38482a"},{"author":{"_account_id":4694,"name":"Miguel Lavalle","email":"miguel@mlavalle.com","username":"minsel"},"change_message_id":"6fc03d7306803028d3007c867b4d1a84a8bb1738","unresolved":true,"context_lines":[{"line_number":380,"context_line":"                           if ovn_const.OVN_SG_RULE_EXT_ID_KEY in oa]"},{"line_number":381,"context_line":"            # We must copy here since we need to keep the original"},{"line_number":382,"context_line":"            # \u0027add_acl\u0027 intact for removal"},{"line_number":383,"context_line":"            for add_acl in add_acls:"},{"line_number":384,"context_line":"                add_acl_copy \u003d copy_acl_rem_id_key(add_acl)"},{"line_number":385,"context_line":"                if add_acl_copy in nd_ovn_acls:"},{"line_number":386,"context_line":"                    add_rem_acls.append(add_acl)"},{"line_number":387,"context_line":""},{"line_number":388,"context_line":"            # Remove any of the false-positive ACLs"},{"line_number":389,"context_line":"            LOG.warning(\u0027False-positive ACLs to remove: (%s)\u0027, add_rem_acls)"}],"source_content_type":"text/x-python","patch_set":15,"id":"6dc38223_2c9e742b","line":386,"range":{"start_line":383,"start_character":0,"end_line":386,"end_character":48},"updated":"2025-08-29 22:56:23.000000000","message":"The time complexity of this loop is O(n * m), where where n \u003d len(add_acls), m \u003d len(nd_ovn_acls). In the worst case where m approached n, this becomes O(n ^^ 2). If we are making the assumption that both add_acls and nd_ovn_acls are small lists, then none of this is an issue. And I think that assumption is reasonable, so I\u0027m fine with it.\n\nHowever, if you think that the assumption will not hold in many cases, then we could improve this code using a set instead of a list storing frozensets instead of dictionaries: https://paste.openstack.org/show/bO7flDm3Kbjn9D99dXV0/","commit_id":"079b0af98e031b77a5dc723cc8d39fdfbba3a6d6"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"eaa141f74bfd57c900eac363259b6b37be3bd8c1","unresolved":true,"context_lines":[{"line_number":380,"context_line":"                           if ovn_const.OVN_SG_RULE_EXT_ID_KEY in oa]"},{"line_number":381,"context_line":"            # We must copy here since we need to keep the original"},{"line_number":382,"context_line":"            # \u0027add_acl\u0027 intact for removal"},{"line_number":383,"context_line":"            for add_acl in add_acls:"},{"line_number":384,"context_line":"                add_acl_copy \u003d copy_acl_rem_id_key(add_acl)"},{"line_number":385,"context_line":"                if add_acl_copy in nd_ovn_acls:"},{"line_number":386,"context_line":"                    add_rem_acls.append(add_acl)"},{"line_number":387,"context_line":""},{"line_number":388,"context_line":"            # Remove any of the false-positive ACLs"},{"line_number":389,"context_line":"            LOG.warning(\u0027False-positive ACLs to remove: (%s)\u0027, add_rem_acls)"}],"source_content_type":"text/x-python","patch_set":15,"id":"9f7ce668_066f97e3","line":386,"range":{"start_line":383,"start_character":0,"end_line":386,"end_character":48},"in_reply_to":"2d9b6237_01996fe6","updated":"2025-09-05 20:29:08.000000000","message":"So I tried many times and couldn\u0027t get this to work yet with your paste so put back my old version, maybe the acl is not conducive to using a frozenset? TypeError: unhashable type: \u0027list\u0027 and I can\u0027t remember the details of what\u0027s in the dict here, would have to run more tests.","commit_id":"079b0af98e031b77a5dc723cc8d39fdfbba3a6d6"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"e9f857377fd4f877951fb554beeaf9429a2eb144","unresolved":true,"context_lines":[{"line_number":380,"context_line":"                           if ovn_const.OVN_SG_RULE_EXT_ID_KEY in oa]"},{"line_number":381,"context_line":"            # We must copy here since we need to keep the original"},{"line_number":382,"context_line":"            # \u0027add_acl\u0027 intact for removal"},{"line_number":383,"context_line":"            for add_acl in add_acls:"},{"line_number":384,"context_line":"                add_acl_copy \u003d copy_acl_rem_id_key(add_acl)"},{"line_number":385,"context_line":"                if add_acl_copy in nd_ovn_acls:"},{"line_number":386,"context_line":"                    add_rem_acls.append(add_acl)"},{"line_number":387,"context_line":""},{"line_number":388,"context_line":"            # Remove any of the false-positive ACLs"},{"line_number":389,"context_line":"            LOG.warning(\u0027False-positive ACLs to remove: (%s)\u0027, add_rem_acls)"}],"source_content_type":"text/x-python","patch_set":15,"id":"2d9b6237_01996fe6","line":386,"range":{"start_line":383,"start_character":0,"end_line":386,"end_character":48},"in_reply_to":"6dc38223_2c9e742b","updated":"2025-09-05 00:53:11.000000000","message":"Hi Miguel, thanks for taking a look. I will admit this loop is non-optimal, my earlier version was even worse.\n\nI guess we can\u0027t assume the lists will be small, users have thousands of SG rules sometimes. I didn\u0027t know about frozensets, let me do a quick replacement test and see what the CI thinks. Thanks!","commit_id":"079b0af98e031b77a5dc723cc8d39fdfbba3a6d6"},{"author":{"_account_id":4694,"name":"Miguel Lavalle","email":"miguel@mlavalle.com","username":"minsel"},"change_message_id":"01a0b1dac2cba5e1454b925322827380fca8f5c2","unresolved":false,"context_lines":[{"line_number":380,"context_line":"                           if ovn_const.OVN_SG_RULE_EXT_ID_KEY in oa]"},{"line_number":381,"context_line":"            # We must copy here since we need to keep the original"},{"line_number":382,"context_line":"            # \u0027add_acl\u0027 intact for removal"},{"line_number":383,"context_line":"            for add_acl in add_acls:"},{"line_number":384,"context_line":"                add_acl_copy \u003d copy_acl_rem_id_key(add_acl)"},{"line_number":385,"context_line":"                if add_acl_copy in nd_ovn_acls:"},{"line_number":386,"context_line":"                    add_rem_acls.append(add_acl)"},{"line_number":387,"context_line":""},{"line_number":388,"context_line":"            # Remove any of the false-positive ACLs"},{"line_number":389,"context_line":"            LOG.warning(\u0027False-positive ACLs to remove: (%s)\u0027, add_rem_acls)"}],"source_content_type":"text/x-python","patch_set":15,"id":"65007321_19aed526","line":386,"range":{"start_line":383,"start_character":0,"end_line":386,"end_character":48},"in_reply_to":"9f7ce668_066f97e3","updated":"2025-09-06 21:48:17.000000000","message":"OK, thanks for trying!","commit_id":"079b0af98e031b77a5dc723cc8d39fdfbba3a6d6"}]}