)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"1405a97bb63788d98a8e425bae4bc319df2aea0d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"66ff2013_69d23269","updated":"2022-10-17 09:25:55.000000000","message":"only some nits. Besides that LGTM. Thx for the patch.","commit_id":"cdc5f0dbff8e8ab01b8424dd217c7bfc8671bbca"},{"author":{"_account_id":8313,"name":"Lajos Katona","display_name":"lajoskatona","email":"katonalala@gmail.com","username":"elajkat","status":"Ericsson Software Technology"},"change_message_id":"5d177c5f9110663d9d78dcf27488adb82c6b1fe1","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"92022d44_7b1921f7","updated":"2022-11-11 15:34:14.000000000","message":"Looks ok, but somebody with more OVN experience should check also","commit_id":"c69ba4bb6ea11df8be41d826f431de8e75aa4877"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"151352ac92fe7e3243dc1515d47fcafcfae3f626","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"005d05d2_dad206d7","updated":"2022-12-06 22:42:12.000000000","message":"Ihar and Hailun,\nMy patch has been going long and I had to recently resolve conflicts against your recent patches to OVN metadata agent. I have tried retaining your intended patch purposes and I want your +/-1 on it. Here is the history of patches I had to resolve against\nhttps://opendev.org/openstack/neutron/commits/branch/master/neutron/agent/ovn/metadata/agent.py\nThanks! ","commit_id":"44aca3ceff4b57dc3e78addcd8db1a90e1ac5dce"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"b92261a254f549d0f1ce1d1205aa78f2485904e3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"58f37383_fda4b4ed","updated":"2022-12-14 15:37:21.000000000","message":"LGTM","commit_id":"44aca3ceff4b57dc3e78addcd8db1a90e1ac5dce"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"b8758c2483fc5f9a22b784168b6b962801132976","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"ce9ae2c6_acf5d1cd","updated":"2022-12-14 16:02:16.000000000","message":"Looks OK","commit_id":"44aca3ceff4b57dc3e78addcd8db1a90e1ac5dce"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"d3b1c3ad8f135fc937424d83ac15d441422ed3b3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"45ddf18f_f834ce7f","updated":"2022-12-07 08:24:04.000000000","message":"Seems to be a smart change. Thanks ++","commit_id":"44aca3ceff4b57dc3e78addcd8db1a90e1ac5dce"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"f7704c89c4585a47ab928387673a3d618b22341b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"0d079f2b_785f5b6e","updated":"2022-12-15 18:52:30.000000000","message":"This code is fine; I left some comments, all minor. You can consider them or leave, it\u0027s not too important.","commit_id":"44aca3ceff4b57dc3e78addcd8db1a90e1ac5dce"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"52a4886346aa804cede8f912de489abc19a7a8e6","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"6bc55dc9_72e995c6","updated":"2023-01-11 15:12:28.000000000","message":"LGTM","commit_id":"edf48e46a1f0227f84b05ab39da005393e5fa73f"},{"author":{"_account_id":8313,"name":"Lajos Katona","display_name":"lajoskatona","email":"katonalala@gmail.com","username":"elajkat","status":"Ericsson Software Technology"},"change_message_id":"c4b6f0500982b42d2594bfdfc1d9d432e4cce8f7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"00c58348_6580d8b2","updated":"2023-01-11 15:17:29.000000000","message":"Nice","commit_id":"edf48e46a1f0227f84b05ab39da005393e5fa73f"}],"neutron/agent/ovn/metadata/agent.py":[{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"29af2b9950bfe77b8aacd1e37b8f9e73ac9eba9c","unresolved":true,"context_lines":[{"line_number":395,"context_line":"        iptables_mgr.ipv4[\u0027mangle\u0027].add_rule(\u0027POSTROUTING\u0027, rule, wrap\u003dFalse)"},{"line_number":396,"context_line":"        iptables_mgr.apply()"},{"line_number":397,"context_line":""},{"line_number":398,"context_line":"    def _get_port_ip(self, port):"},{"line_number":399,"context_line":"        # FIXME(Miro): This assumption is very fragile"},{"line_number":400,"context_line":"        # Find more determistic way to get a port IP."},{"line_number":401,"context_line":"        # Retreive IP from mac colum which is in form"},{"line_number":402,"context_line":"        # [\"\u003cmac\u003e \u003cip\u003e \u003cip2\u003e ...\"\"]"},{"line_number":403,"context_line":"        mac_field_attrs \u003d port.mac[0].split(\u0027 \u0027)"},{"line_number":404,"context_line":"        if len(mac_field_attrs) \u003e 1:"},{"line_number":405,"context_line":"            return mac_field_attrs[1]"},{"line_number":406,"context_line":"        LOG.debug(\"Port %s IP address was not retrieved from the \""},{"line_number":407,"context_line":"                  \"Port_Binding MAC column %s\", port.uuid, mac_field_attrs)"},{"line_number":408,"context_line":"        return"},{"line_number":409,"context_line":""},{"line_number":410,"context_line":"    def _active_subnets_cidrs(self, datapath_ports_ips, metadata_port_cidrs):"},{"line_number":411,"context_line":"        active_subnets_cidrs \u003d set()"}],"source_content_type":"text/x-python","patch_set":1,"id":"442fbfa8_a0e83b13","line":408,"range":{"start_line":398,"start_character":1,"end_line":408,"end_character":14},"updated":"2022-10-13 17:09:21.000000000","message":"FYI, I will not submit this patch with this FIXME. This approach is just temporary. I want to consult with some OVN experts on what would be the best way to retrieve port\u0027s IPv4 address. Here is a record of a port from the Port_Binding table.\n\nhttps://paste.opendev.org/show/817162/\n\nThis port has both ipv4 and ipv6 IPs (belongs to ipv4 and ipv6 subnet). In my current temporary approach, I just grab the first IP after the MAC address from the mac column. Its usually the IPV4 address of the VM or it could be just IPv6 address if VM is only connected to a ipv6 subnet. \nIs this a good approach? \nShould I make it more robust and maybe perform regex match for the first IPv4 address?\nDo you know a better way to determine port IP? \nThanks!","commit_id":"6b65f33d99f328e7f050bdf4d010d1599052e2b6"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"e234bf543b30e13b3494a2db3567d8eb0b47fa2b","unresolved":false,"context_lines":[{"line_number":395,"context_line":"        iptables_mgr.ipv4[\u0027mangle\u0027].add_rule(\u0027POSTROUTING\u0027, rule, wrap\u003dFalse)"},{"line_number":396,"context_line":"        iptables_mgr.apply()"},{"line_number":397,"context_line":""},{"line_number":398,"context_line":"    def _get_port_ip(self, port):"},{"line_number":399,"context_line":"        # FIXME(Miro): This assumption is very fragile"},{"line_number":400,"context_line":"        # Find more determistic way to get a port IP."},{"line_number":401,"context_line":"        # Retreive IP from mac colum which is in form"},{"line_number":402,"context_line":"        # [\"\u003cmac\u003e \u003cip\u003e \u003cip2\u003e ...\"\"]"},{"line_number":403,"context_line":"        mac_field_attrs \u003d port.mac[0].split(\u0027 \u0027)"},{"line_number":404,"context_line":"        if len(mac_field_attrs) \u003e 1:"},{"line_number":405,"context_line":"            return mac_field_attrs[1]"},{"line_number":406,"context_line":"        LOG.debug(\"Port %s IP address was not retrieved from the \""},{"line_number":407,"context_line":"                  \"Port_Binding MAC column %s\", port.uuid, mac_field_attrs)"},{"line_number":408,"context_line":"        return"},{"line_number":409,"context_line":""},{"line_number":410,"context_line":"    def _active_subnets_cidrs(self, datapath_ports_ips, metadata_port_cidrs):"},{"line_number":411,"context_line":"        active_subnets_cidrs \u003d set()"}],"source_content_type":"text/x-python","patch_set":1,"id":"2a774429_74a80641","line":408,"range":{"start_line":398,"start_character":1,"end_line":408,"end_character":14},"in_reply_to":"442fbfa8_a0e83b13","updated":"2022-10-27 02:36:22.000000000","message":"Rodolfo started conversation about this topic in a separate comment. I will close this comment so there are no duplicate threads about the same topic","commit_id":"6b65f33d99f328e7f050bdf4d010d1599052e2b6"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"59344e9e44892423473cf8a5af61699b113ac544","unresolved":true,"context_lines":[{"line_number":438,"context_line":""},{"line_number":439,"context_line":"        Function will confirm that:"},{"line_number":440,"context_line":"        1. There are datapath port IPs"},{"line_number":441,"context_line":"        2. Datapath port has valid MAC and subnet CIDRs"},{"line_number":442,"context_line":""},{"line_number":443,"context_line":"        If any of those rules are not valid the nemaspace for the"},{"line_number":444,"context_line":"        provided datapath will be tore down."}],"source_content_type":"text/x-python","patch_set":1,"id":"a975ff85_a5197e5f","line":441,"range":{"start_line":441,"start_character":11,"end_line":441,"end_character":19},"updated":"2022-10-20 02:35:07.000000000","message":"This should say \"Datapath metadata port ... \"","commit_id":"6b65f33d99f328e7f050bdf4d010d1599052e2b6"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"8d4edf690845559ac2246929d045c28714531f99","unresolved":false,"context_lines":[{"line_number":438,"context_line":""},{"line_number":439,"context_line":"        Function will confirm that:"},{"line_number":440,"context_line":"        1. There are datapath port IPs"},{"line_number":441,"context_line":"        2. Datapath port has valid MAC and subnet CIDRs"},{"line_number":442,"context_line":""},{"line_number":443,"context_line":"        If any of those rules are not valid the nemaspace for the"},{"line_number":444,"context_line":"        provided datapath will be tore down."}],"source_content_type":"text/x-python","patch_set":1,"id":"2c01d173_d9c5491d","line":441,"range":{"start_line":441,"start_character":11,"end_line":441,"end_character":19},"in_reply_to":"a975ff85_a5197e5f","updated":"2022-10-21 02:15:11.000000000","message":"Done","commit_id":"6b65f33d99f328e7f050bdf4d010d1599052e2b6"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"3fecf458d2d560a8293bc98e843de730730d376f","unresolved":true,"context_lines":[{"line_number":550,"context_line":"            metadata_port_info.ip_addresses"},{"line_number":551,"context_line":"        )"},{"line_number":552,"context_line":"        # Delete any non active addresses from the network namespace"},{"line_number":553,"context_line":"        for cidr in cidrs_to_delete:"},{"line_number":554,"context_line":"            ip2.addr.delete(cidr)"},{"line_number":555,"context_line":"        for cidr in cidrs_to_add:"},{"line_number":556,"context_line":"            # NOTE(dalvarez): metadata only works on IPv4. We\u0027re doing this"},{"line_number":557,"context_line":"            # extra check here because it could be that the metadata port has"},{"line_number":558,"context_line":"            # an IPv6 address if there\u0027s an IPv6 subnet with SLAAC in its"}],"source_content_type":"text/x-python","patch_set":1,"id":"17b0da56_08d8c456","line":555,"range":{"start_line":553,"start_character":0,"end_line":555,"end_character":33},"updated":"2022-10-13 17:40:19.000000000","message":"FYI, I will rebase this (or merge resolve) this with my change from here\nhttps://review.opendev.org/c/openstack/neutron/+/855677\n\nThis way, we will just pass list of cidrs to delete and cidrs to add. This should have slightly better performance as it only needs to setup two privileged calls as discussed in patch 855677","commit_id":"6b65f33d99f328e7f050bdf4d010d1599052e2b6"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"e234bf543b30e13b3494a2db3567d8eb0b47fa2b","unresolved":false,"context_lines":[{"line_number":550,"context_line":"            metadata_port_info.ip_addresses"},{"line_number":551,"context_line":"        )"},{"line_number":552,"context_line":"        # Delete any non active addresses from the network namespace"},{"line_number":553,"context_line":"        for cidr in cidrs_to_delete:"},{"line_number":554,"context_line":"            ip2.addr.delete(cidr)"},{"line_number":555,"context_line":"        for cidr in cidrs_to_add:"},{"line_number":556,"context_line":"            # NOTE(dalvarez): metadata only works on IPv4. We\u0027re doing this"},{"line_number":557,"context_line":"            # extra check here because it could be that the metadata port has"},{"line_number":558,"context_line":"            # an IPv6 address if there\u0027s an IPv6 subnet with SLAAC in its"}],"source_content_type":"text/x-python","patch_set":1,"id":"b46a7a49_8b1f625f","line":555,"range":{"start_line":553,"start_character":0,"end_line":555,"end_character":33},"in_reply_to":"17b0da56_08d8c456","updated":"2022-10-27 02:36:22.000000000","message":"Ack","commit_id":"6b65f33d99f328e7f050bdf4d010d1599052e2b6"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"161ab2708878f158313a74c310a968f58a71e5f0","unresolved":true,"context_lines":[{"line_number":400,"context_line":"        # Find more determistic way to get a port IP."},{"line_number":401,"context_line":"        # Retreive IP from mac colum which is in form"},{"line_number":402,"context_line":"        # [\"\u003cmac\u003e \u003cip\u003e \u003cip2\u003e ...\"\"]"},{"line_number":403,"context_line":"        mac_field_attrs \u003d port.mac[0].split(\u0027 \u0027)"},{"line_number":404,"context_line":"        if len(mac_field_attrs) \u003e 1:"},{"line_number":405,"context_line":"            return mac_field_attrs[1]"},{"line_number":406,"context_line":"        LOG.debug(\"Port %s IP address was not retrieved from the \""}],"source_content_type":"text/x-python","patch_set":2,"id":"30e55001_5a651f4e","line":403,"range":{"start_line":403,"start_character":8,"end_line":403,"end_character":48},"updated":"2022-10-18 07:45:44.000000000","message":"We store the port CIDRs in the external IDs. You have an example of this in [1]: how to retrieve the CIDRs (not only one but several) and the IPs.\n\n[1]https://github.com/openstack/neutron/blob/799c66c2e7c47d505bd6335c392c6d0d014539e8/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py#L4077-L4079","commit_id":"cdc5f0dbff8e8ab01b8424dd217c7bfc8671bbca"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"ea3137b8845946073a79e9423e1b7f22048f1a42","unresolved":false,"context_lines":[{"line_number":400,"context_line":"        # Find more determistic way to get a port IP."},{"line_number":401,"context_line":"        # Retreive IP from mac colum which is in form"},{"line_number":402,"context_line":"        # [\"\u003cmac\u003e \u003cip\u003e \u003cip2\u003e ...\"\"]"},{"line_number":403,"context_line":"        mac_field_attrs \u003d port.mac[0].split(\u0027 \u0027)"},{"line_number":404,"context_line":"        if len(mac_field_attrs) \u003e 1:"},{"line_number":405,"context_line":"            return mac_field_attrs[1]"},{"line_number":406,"context_line":"        LOG.debug(\"Port %s IP address was not retrieved from the \""}],"source_content_type":"text/x-python","patch_set":2,"id":"d7b4854a_154ff0a9","line":403,"range":{"start_line":403,"start_character":8,"end_line":403,"end_character":48},"in_reply_to":"30e3daab_4c873701","updated":"2022-11-11 15:30:07.000000000","message":"I won\u0027t nitpick this patch because of this.","commit_id":"cdc5f0dbff8e8ab01b8424dd217c7bfc8671bbca"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"59344e9e44892423473cf8a5af61699b113ac544","unresolved":true,"context_lines":[{"line_number":400,"context_line":"        # Find more determistic way to get a port IP."},{"line_number":401,"context_line":"        # Retreive IP from mac colum which is in form"},{"line_number":402,"context_line":"        # [\"\u003cmac\u003e \u003cip\u003e \u003cip2\u003e ...\"\"]"},{"line_number":403,"context_line":"        mac_field_attrs \u003d port.mac[0].split(\u0027 \u0027)"},{"line_number":404,"context_line":"        if len(mac_field_attrs) \u003e 1:"},{"line_number":405,"context_line":"            return mac_field_attrs[1]"},{"line_number":406,"context_line":"        LOG.debug(\"Port %s IP address was not retrieved from the \""}],"source_content_type":"text/x-python","patch_set":2,"id":"775af28e_a6f63452","line":403,"range":{"start_line":403,"start_character":8,"end_line":403,"end_character":48},"in_reply_to":"30e55001_5a651f4e","updated":"2022-10-20 02:35:07.000000000","message":"Thanks. This was actually my question, I made comment about it further up.\n\nIt looks like I can also retrieve all IPs from mac column. Wouldn\u0027t that be little easier? Or are those IPs are not guarantee to be in MAC column? Here is a record from Port_Binding table\n\nhttps://paste.opendev.org/show/817162/\n\nI can do something like this:\nport_ips \u003d port.mac[0].split(\u0027 \u0027)[1:]  # of course I would do this more safely\n\nOr what you pointed out:\nport_ips \u003d [str(netaddr.IPNetwork(cidr).ip)\n     for cidr in port.external_ids.get(ovn_const.OVN_CIDRS_EXT_ID_KEY, \u0027\u0027).split()\n]","commit_id":"cdc5f0dbff8e8ab01b8424dd217c7bfc8671bbca"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"e234bf543b30e13b3494a2db3567d8eb0b47fa2b","unresolved":true,"context_lines":[{"line_number":400,"context_line":"        # Find more determistic way to get a port IP."},{"line_number":401,"context_line":"        # Retreive IP from mac colum which is in form"},{"line_number":402,"context_line":"        # [\"\u003cmac\u003e \u003cip\u003e \u003cip2\u003e ...\"\"]"},{"line_number":403,"context_line":"        mac_field_attrs \u003d port.mac[0].split(\u0027 \u0027)"},{"line_number":404,"context_line":"        if len(mac_field_attrs) \u003e 1:"},{"line_number":405,"context_line":"            return mac_field_attrs[1]"},{"line_number":406,"context_line":"        LOG.debug(\"Port %s IP address was not retrieved from the \""}],"source_content_type":"text/x-python","patch_set":2,"id":"30e3daab_4c873701","line":403,"range":{"start_line":403,"start_character":8,"end_line":403,"end_character":48},"in_reply_to":"775af28e_a6f63452","updated":"2022-10-27 02:36:22.000000000","message":"my latest update contains implementation where IPs are retrieved from mac port. Let me know what you think about that. I can change it to \"external ids\" implementation if you think its safer.","commit_id":"cdc5f0dbff8e8ab01b8424dd217c7bfc8671bbca"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"161ab2708878f158313a74c310a968f58a71e5f0","unresolved":true,"context_lines":[{"line_number":431,"context_line":""},{"line_number":432,"context_line":"        cidrs_to_delete \u003d current_namespace_cidrs - active_subnets_cidrs"},{"line_number":433,"context_line":""},{"line_number":434,"context_line":"        return (cidrs_to_add, cidrs_to_delete,)"},{"line_number":435,"context_line":""},{"line_number":436,"context_line":"    def _preprovision_checks(self, datapath, datapath_ports_ips\u003dNone):"},{"line_number":437,"context_line":"        \"\"\"Performs datapath preprovision checks"}],"source_content_type":"text/x-python","patch_set":2,"id":"284852a0_8466d665","line":434,"range":{"start_line":434,"start_character":45,"end_line":434,"end_character":46},"updated":"2022-10-18 07:45:44.000000000","message":"nit: not needed","commit_id":"cdc5f0dbff8e8ab01b8424dd217c7bfc8671bbca"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"59344e9e44892423473cf8a5af61699b113ac544","unresolved":false,"context_lines":[{"line_number":431,"context_line":""},{"line_number":432,"context_line":"        cidrs_to_delete \u003d current_namespace_cidrs - active_subnets_cidrs"},{"line_number":433,"context_line":""},{"line_number":434,"context_line":"        return (cidrs_to_add, cidrs_to_delete,)"},{"line_number":435,"context_line":""},{"line_number":436,"context_line":"    def _preprovision_checks(self, datapath, datapath_ports_ips\u003dNone):"},{"line_number":437,"context_line":"        \"\"\"Performs datapath preprovision checks"}],"source_content_type":"text/x-python","patch_set":2,"id":"04aeeca6_c9e71e54","line":434,"range":{"start_line":434,"start_character":45,"end_line":434,"end_character":46},"in_reply_to":"284852a0_8466d665","updated":"2022-10-20 02:35:07.000000000","message":"Ack","commit_id":"cdc5f0dbff8e8ab01b8424dd217c7bfc8671bbca"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"161ab2708878f158313a74c310a968f58a71e5f0","unresolved":true,"context_lines":[{"line_number":455,"context_line":"            datapath_ports_ips \u003d []"},{"line_number":456,"context_line":"            for chassis_port in self._vif_ports(chassis_ports):"},{"line_number":457,"context_line":"                if str(chassis_port.datapath.uuid) \u003d\u003d datapath_uuid:"},{"line_number":458,"context_line":"                    port_ip \u003d self._get_port_ip(chassis_port)"},{"line_number":459,"context_line":"                    if port_ip:"},{"line_number":460,"context_line":"                        datapath_ports_ips.append(port_ip)"},{"line_number":461,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"0e1e9639_58c2aecc","line":458,"range":{"start_line":458,"start_character":20,"end_line":458,"end_character":61},"updated":"2022-10-18 07:45:44.000000000","message":"We can have several IP addresses per port.","commit_id":"cdc5f0dbff8e8ab01b8424dd217c7bfc8671bbca"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"e234bf543b30e13b3494a2db3567d8eb0b47fa2b","unresolved":false,"context_lines":[{"line_number":455,"context_line":"            datapath_ports_ips \u003d []"},{"line_number":456,"context_line":"            for chassis_port in self._vif_ports(chassis_ports):"},{"line_number":457,"context_line":"                if str(chassis_port.datapath.uuid) \u003d\u003d datapath_uuid:"},{"line_number":458,"context_line":"                    port_ip \u003d self._get_port_ip(chassis_port)"},{"line_number":459,"context_line":"                    if port_ip:"},{"line_number":460,"context_line":"                        datapath_ports_ips.append(port_ip)"},{"line_number":461,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"eb57aa71_0a37ed54","line":458,"range":{"start_line":458,"start_character":20,"end_line":458,"end_character":61},"in_reply_to":"0e1e9639_58c2aecc","updated":"2022-10-27 02:36:22.000000000","message":"Done","commit_id":"cdc5f0dbff8e8ab01b8424dd217c7bfc8671bbca"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"161ab2708878f158313a74c310a968f58a71e5f0","unresolved":true,"context_lines":[{"line_number":485,"context_line":"            LOG.error(\"Metadata port for network %s doesn\u0027t have a MAC \""},{"line_number":486,"context_line":"                      \"address, tearing the namespace down if needed\","},{"line_number":487,"context_line":"                      net_name)"},{"line_number":488,"context_line":"            self.teardown_datapath(net_name)"},{"line_number":489,"context_line":"            return"},{"line_number":490,"context_line":""},{"line_number":491,"context_line":"        mac \u003d match.group()"}],"source_content_type":"text/x-python","patch_set":2,"id":"df29db85_8ff5e779","line":488,"range":{"start_line":488,"start_character":12,"end_line":488,"end_character":44},"updated":"2022-10-18 07:45:44.000000000","message":"We teardown the datapath if:\n1) There is no IP on the metadata port\n2) The port doesn\u0027t have MAC\n3) (new) There are no \"datapath_ports_ips\"\n\nThe last one (3) is the more expensive to test but executed before (1) and (2). Why don\u0027t you move it here?","commit_id":"cdc5f0dbff8e8ab01b8424dd217c7bfc8671bbca"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"59344e9e44892423473cf8a5af61699b113ac544","unresolved":false,"context_lines":[{"line_number":485,"context_line":"            LOG.error(\"Metadata port for network %s doesn\u0027t have a MAC \""},{"line_number":486,"context_line":"                      \"address, tearing the namespace down if needed\","},{"line_number":487,"context_line":"                      net_name)"},{"line_number":488,"context_line":"            self.teardown_datapath(net_name)"},{"line_number":489,"context_line":"            return"},{"line_number":490,"context_line":""},{"line_number":491,"context_line":"        mac \u003d match.group()"}],"source_content_type":"text/x-python","patch_set":2,"id":"5cdaacda_d8dd7b14","line":488,"range":{"start_line":488,"start_character":12,"end_line":488,"end_character":44},"in_reply_to":"df29db85_8ff5e779","updated":"2022-10-20 02:35:07.000000000","message":"I think I did that to stay with \"flow\" of the original code. Notice, I completely deleted `update_datapath` method which simply checked if there are datapath_ports, if not it tore down datapath. I cleaned up all these checks to my new method `_preprovision_checks` so the code is cleaner and easier to unit tests.\n\nRegardless, the order doesn\u0027t matter. And I actually think your feedback is good because getting datapath_port IPs is the most processor heavy.","commit_id":"cdc5f0dbff8e8ab01b8424dd217c7bfc8671bbca"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"161ab2708878f158313a74c310a968f58a71e5f0","unresolved":true,"context_lines":[{"line_number":495,"context_line":"        metadata_port_info \u003d MetadataPortInfo(mac, ip_addresses,"},{"line_number":496,"context_line":"                                              metadata_port.logical_port)"},{"line_number":497,"context_line":""},{"line_number":498,"context_line":"        return (net_name, datapath_ports_ips, metadata_port_info,)"},{"line_number":499,"context_line":""},{"line_number":500,"context_line":"    def provision_datapath(self, datapath, datapath_ports_ips\u003dNone):"},{"line_number":501,"context_line":"        \"\"\"Provision the datapath so that it can serve metadata."}],"source_content_type":"text/x-python","patch_set":2,"id":"a24df3b1_ee97f4e6","line":498,"range":{"start_line":498,"start_character":64,"end_line":498,"end_character":65},"updated":"2022-10-18 07:45:44.000000000","message":"nit: not needed","commit_id":"cdc5f0dbff8e8ab01b8424dd217c7bfc8671bbca"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"59344e9e44892423473cf8a5af61699b113ac544","unresolved":false,"context_lines":[{"line_number":495,"context_line":"        metadata_port_info \u003d MetadataPortInfo(mac, ip_addresses,"},{"line_number":496,"context_line":"                                              metadata_port.logical_port)"},{"line_number":497,"context_line":""},{"line_number":498,"context_line":"        return (net_name, datapath_ports_ips, metadata_port_info,)"},{"line_number":499,"context_line":""},{"line_number":500,"context_line":"    def provision_datapath(self, datapath, datapath_ports_ips\u003dNone):"},{"line_number":501,"context_line":"        \"\"\"Provision the datapath so that it can serve metadata."}],"source_content_type":"text/x-python","patch_set":2,"id":"677b5f7b_746f73b7","line":498,"range":{"start_line":498,"start_character":64,"end_line":498,"end_character":65},"in_reply_to":"a24df3b1_ee97f4e6","updated":"2022-10-20 02:35:07.000000000","message":"Ack","commit_id":"cdc5f0dbff8e8ab01b8424dd217c7bfc8671bbca"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"1405a97bb63788d98a8e425bae4bc319df2aea0d","unresolved":true,"context_lines":[{"line_number":512,"context_line":"                 if namespace was not provisioned"},{"line_number":513,"context_line":"        \"\"\""},{"line_number":514,"context_line":""},{"line_number":515,"context_line":"        provision_params \u003d self._preprovision_checks(datapath,"},{"line_number":516,"context_line":"                                                     datapath_ports_ips)"},{"line_number":517,"context_line":"        if not provision_params:"},{"line_number":518,"context_line":"            return"}],"source_content_type":"text/x-python","patch_set":2,"id":"17643534_c9cd35c9","line":515,"range":{"start_line":515,"start_character":32,"end_line":515,"end_character":52},"updated":"2022-10-17 09:25:55.000000000","message":"nit: I would name it like \"_get_provision_params\" or something like that as \"_checks...\" means for me that it will just perform some checks and not return anything.","commit_id":"cdc5f0dbff8e8ab01b8424dd217c7bfc8671bbca"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"59344e9e44892423473cf8a5af61699b113ac544","unresolved":false,"context_lines":[{"line_number":512,"context_line":"                 if namespace was not provisioned"},{"line_number":513,"context_line":"        \"\"\""},{"line_number":514,"context_line":""},{"line_number":515,"context_line":"        provision_params \u003d self._preprovision_checks(datapath,"},{"line_number":516,"context_line":"                                                     datapath_ports_ips)"},{"line_number":517,"context_line":"        if not provision_params:"},{"line_number":518,"context_line":"            return"}],"source_content_type":"text/x-python","patch_set":2,"id":"14833b07_5216a743","line":515,"range":{"start_line":515,"start_character":32,"end_line":515,"end_character":52},"in_reply_to":"17643534_c9cd35c9","updated":"2022-10-20 02:35:07.000000000","message":"Ack","commit_id":"cdc5f0dbff8e8ab01b8424dd217c7bfc8671bbca"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"1405a97bb63788d98a8e425bae4bc319df2aea0d","unresolved":true,"context_lines":[{"line_number":557,"context_line":"            # extra check here because it could be that the metadata port has"},{"line_number":558,"context_line":"            # an IPv6 address if there\u0027s an IPv6 subnet with SLAAC in its"},{"line_number":559,"context_line":"            # network. Neutron IPAM will autoallocate an IPv6 address for every"},{"line_number":560,"context_line":"            # port in the network."},{"line_number":561,"context_line":"            if utils.get_ip_version(cidr) \u003d\u003d 4:"},{"line_number":562,"context_line":"                ip2.addr.add(cidr)"},{"line_number":563,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"bc1793a1_4bf21aae","line":560,"updated":"2022-10-17 09:25:55.000000000","message":"not really related to that patch but this comment is outdated - Neutron supports metadata for IPv6 also. We should probably open new LP to add support for it in ovn backend too.","commit_id":"cdc5f0dbff8e8ab01b8424dd217c7bfc8671bbca"},{"author":{"_account_id":8313,"name":"Lajos Katona","display_name":"lajoskatona","email":"katonalala@gmail.com","username":"elajkat","status":"Ericsson Software Technology"},"change_message_id":"5d177c5f9110663d9d78dcf27488adb82c6b1fe1","unresolved":true,"context_lines":[{"line_number":557,"context_line":"            # extra check here because it could be that the metadata port has"},{"line_number":558,"context_line":"            # an IPv6 address if there\u0027s an IPv6 subnet with SLAAC in its"},{"line_number":559,"context_line":"            # network. Neutron IPAM will autoallocate an IPv6 address for every"},{"line_number":560,"context_line":"            # port in the network."},{"line_number":561,"context_line":"            if utils.get_ip_version(cidr) \u003d\u003d 4:"},{"line_number":562,"context_line":"                ip2.addr.add(cidr)"},{"line_number":563,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"c15888ba_5b896a9f","line":560,"in_reply_to":"419bb678_f81d8942","updated":"2022-11-11 15:34:14.000000000","message":"For the legacy metadata IPv6 works (since: https://review.opendev.org/q/topic:metadata-ipv6 ), but to tell the truth not sure about how with OVN works, I tried to find if it is also works, and it is not in the gaps list (https://docs.openstack.org/neutron/latest/ovn/gaps.html )","commit_id":"cdc5f0dbff8e8ab01b8424dd217c7bfc8671bbca"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"59344e9e44892423473cf8a5af61699b113ac544","unresolved":true,"context_lines":[{"line_number":557,"context_line":"            # extra check here because it could be that the metadata port has"},{"line_number":558,"context_line":"            # an IPv6 address if there\u0027s an IPv6 subnet with SLAAC in its"},{"line_number":559,"context_line":"            # network. Neutron IPAM will autoallocate an IPv6 address for every"},{"line_number":560,"context_line":"            # port in the network."},{"line_number":561,"context_line":"            if utils.get_ip_version(cidr) \u003d\u003d 4:"},{"line_number":562,"context_line":"                ip2.addr.add(cidr)"},{"line_number":563,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"419bb678_f81d8942","line":560,"in_reply_to":"bc1793a1_4bf21aae","updated":"2022-10-20 02:35:07.000000000","message":"Yeah I was not sure about this comment when I read it. So what is still missing to get IPv6 to work? That\u0027s not very clear to me.\nOnce, I clarify that I will open LP and update this comment.\n\nThanks","commit_id":"cdc5f0dbff8e8ab01b8424dd217c7bfc8671bbca"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"1405a97bb63788d98a8e425bae4bc319df2aea0d","unresolved":true,"context_lines":[{"line_number":558,"context_line":"            # an IPv6 address if there\u0027s an IPv6 subnet with SLAAC in its"},{"line_number":559,"context_line":"            # network. Neutron IPAM will autoallocate an IPv6 address for every"},{"line_number":560,"context_line":"            # port in the network."},{"line_number":561,"context_line":"            if utils.get_ip_version(cidr) \u003d\u003d 4:"},{"line_number":562,"context_line":"                ip2.addr.add(cidr)"},{"line_number":563,"context_line":""},{"line_number":564,"context_line":"        # Check that this port is not attached to any other OVS bridge. This"}],"source_content_type":"text/x-python","patch_set":2,"id":"ecfb803f_c5c4a4cb","line":561,"range":{"start_line":561,"start_character":45,"end_line":561,"end_character":46},"updated":"2022-10-17 09:25:55.000000000","message":"nit: I know it\u0027s not Your patch really but can You use constant from neutron_lib here?","commit_id":"cdc5f0dbff8e8ab01b8424dd217c7bfc8671bbca"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"59344e9e44892423473cf8a5af61699b113ac544","unresolved":false,"context_lines":[{"line_number":558,"context_line":"            # an IPv6 address if there\u0027s an IPv6 subnet with SLAAC in its"},{"line_number":559,"context_line":"            # network. Neutron IPAM will autoallocate an IPv6 address for every"},{"line_number":560,"context_line":"            # port in the network."},{"line_number":561,"context_line":"            if utils.get_ip_version(cidr) \u003d\u003d 4:"},{"line_number":562,"context_line":"                ip2.addr.add(cidr)"},{"line_number":563,"context_line":""},{"line_number":564,"context_line":"        # Check that this port is not attached to any other OVS bridge. This"}],"source_content_type":"text/x-python","patch_set":2,"id":"bee7ac87_d7e40aa4","line":561,"range":{"start_line":561,"start_character":45,"end_line":561,"end_character":46},"in_reply_to":"65564478_4ef86d0c","updated":"2022-10-20 02:35:07.000000000","message":"Ack","commit_id":"cdc5f0dbff8e8ab01b8424dd217c7bfc8671bbca"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"161ab2708878f158313a74c310a968f58a71e5f0","unresolved":true,"context_lines":[{"line_number":558,"context_line":"            # an IPv6 address if there\u0027s an IPv6 subnet with SLAAC in its"},{"line_number":559,"context_line":"            # network. Neutron IPAM will autoallocate an IPv6 address for every"},{"line_number":560,"context_line":"            # port in the network."},{"line_number":561,"context_line":"            if utils.get_ip_version(cidr) \u003d\u003d 4:"},{"line_number":562,"context_line":"                ip2.addr.add(cidr)"},{"line_number":563,"context_line":""},{"line_number":564,"context_line":"        # Check that this port is not attached to any other OVS bridge. This"}],"source_content_type":"text/x-python","patch_set":2,"id":"65564478_4ef86d0c","line":561,"range":{"start_line":561,"start_character":45,"end_line":561,"end_character":46},"in_reply_to":"ecfb803f_c5c4a4cb","updated":"2022-10-18 07:45:44.000000000","message":"Right, please use the n-lib const: neutron_lib.constants.IP_VERSION_4","commit_id":"cdc5f0dbff8e8ab01b8424dd217c7bfc8671bbca"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"8d4edf690845559ac2246929d045c28714531f99","unresolved":true,"context_lines":[{"line_number":410,"context_line":"    def _active_subnets_cidrs(self, datapath_ports_ips, metadata_port_cidrs):"},{"line_number":411,"context_line":"        active_subnets_cidrs \u003d set()"},{"line_number":412,"context_line":"        for datapath_port_ip in datapath_ports_ips:"},{"line_number":413,"context_line":"            ip_obj \u003d netaddr.IPAddress(datapath_port_ip)"},{"line_number":414,"context_line":"            for metadata_cidr in metadata_port_cidrs:"},{"line_number":415,"context_line":"                if ip_obj in netaddr.IPNetwork(metadata_cidr):"},{"line_number":416,"context_line":"                    active_subnets_cidrs.add(metadata_cidr)"},{"line_number":417,"context_line":"                    break"},{"line_number":418,"context_line":"        return active_subnets_cidrs"},{"line_number":419,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"6a74f506_d843376f","line":416,"range":{"start_line":413,"start_character":0,"end_line":416,"end_character":59},"updated":"2022-10-21 02:15:11.000000000","message":"NOTE to SELF: I just discovered netaddr.all_matching_cidrs function which does exactly this already\n\nhttps://netaddr.readthedocs.io/en/latest/api.html?highlight\u003dall_matching_cidrs#netaddr.all_matching_cidrs","commit_id":"eefa3a0e79819bd944f6b9055609d7209edf884f"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"e234bf543b30e13b3494a2db3567d8eb0b47fa2b","unresolved":false,"context_lines":[{"line_number":410,"context_line":"    def _active_subnets_cidrs(self, datapath_ports_ips, metadata_port_cidrs):"},{"line_number":411,"context_line":"        active_subnets_cidrs \u003d set()"},{"line_number":412,"context_line":"        for datapath_port_ip in datapath_ports_ips:"},{"line_number":413,"context_line":"            ip_obj \u003d netaddr.IPAddress(datapath_port_ip)"},{"line_number":414,"context_line":"            for metadata_cidr in metadata_port_cidrs:"},{"line_number":415,"context_line":"                if ip_obj in netaddr.IPNetwork(metadata_cidr):"},{"line_number":416,"context_line":"                    active_subnets_cidrs.add(metadata_cidr)"},{"line_number":417,"context_line":"                    break"},{"line_number":418,"context_line":"        return active_subnets_cidrs"},{"line_number":419,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"2db415d0_c486fc1b","line":416,"range":{"start_line":413,"start_character":0,"end_line":416,"end_character":59},"in_reply_to":"6a74f506_d843376f","updated":"2022-10-27 02:36:22.000000000","message":"Although it will do what I want, I decided to keep my implementation just because I also wanted to have cidrs in string format instead of cidr objects and did not want to deal with yet another loop to convert to strings.","commit_id":"eefa3a0e79819bd944f6b9055609d7209edf884f"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"ea3137b8845946073a79e9423e1b7f22048f1a42","unresolved":false,"context_lines":[{"line_number":513,"context_line":"        \"\"\""},{"line_number":514,"context_line":""},{"line_number":515,"context_line":"        provision_params \u003d self._get_provision_params(datapath,"},{"line_number":516,"context_line":"                                                     datapath_ports_ips)"},{"line_number":517,"context_line":"        if not provision_params:"},{"line_number":518,"context_line":"            return"},{"line_number":519,"context_line":"        net_name, datapath_ports_ips, metadata_port_info \u003d provision_params"}],"source_content_type":"text/x-python","patch_set":4,"id":"710f5143_763a4e4c","line":516,"range":{"start_line":516,"start_character":53,"end_line":516,"end_character":54},"updated":"2022-11-11 15:30:07.000000000","message":"nit: indentation (pep8 should fail in these cases)","commit_id":"c69ba4bb6ea11df8be41d826f431de8e75aa4877"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"f7704c89c4585a47ab928387673a3d618b22341b","unresolved":true,"context_lines":[{"line_number":490,"context_line":"        metadata port of the network. It will also remove existing IP"},{"line_number":491,"context_line":"        addresses that are no longer needed."},{"line_number":492,"context_line":"        \"\"\""},{"line_number":493,"context_line":"        LOG.info(\"Provisioning metadata for network %s\", net_name)"},{"line_number":494,"context_line":"        port \u003d self.sb_idl.get_metadata_port_network(datapath)"},{"line_number":495,"context_line":"        # If there\u0027s no metadata port or it doesn\u0027t have a MAC or IP"},{"line_number":496,"context_line":"        # addresses, then tear the namespace down if needed. This might happen"}],"source_content_type":"text/x-python","patch_set":5,"id":"850c554b_7ce812a1","side":"PARENT","line":493,"updated":"2022-12-15 18:52:30.000000000","message":"this LOG message may still be helpful in the future. Shouldn\u0027t we keep it?","commit_id":"16399a2ce5816c38f4806659d8661e776394d35f"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"960e6ab46b32259b29db41f7a75cc855c03ad91b","unresolved":false,"context_lines":[{"line_number":490,"context_line":"        metadata port of the network. It will also remove existing IP"},{"line_number":491,"context_line":"        addresses that are no longer needed."},{"line_number":492,"context_line":"        \"\"\""},{"line_number":493,"context_line":"        LOG.info(\"Provisioning metadata for network %s\", net_name)"},{"line_number":494,"context_line":"        port \u003d self.sb_idl.get_metadata_port_network(datapath)"},{"line_number":495,"context_line":"        # If there\u0027s no metadata port or it doesn\u0027t have a MAC or IP"},{"line_number":496,"context_line":"        # addresses, then tear the namespace down if needed. This might happen"}],"source_content_type":"text/x-python","patch_set":5,"id":"759fcc0b_83c9e13e","side":"PARENT","line":493,"in_reply_to":"850c554b_7ce812a1","updated":"2022-12-19 14:08:11.000000000","message":"Good catch! I probably dropped it when resolving merge conflicts.","commit_id":"16399a2ce5816c38f4806659d8661e776394d35f"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"f7704c89c4585a47ab928387673a3d618b22341b","unresolved":true,"context_lines":[{"line_number":464,"context_line":"        iptables_mgr.apply()"},{"line_number":465,"context_line":""},{"line_number":466,"context_line":"    def _get_port_ips(self, port):"},{"line_number":467,"context_line":"        # Retreive IPs from the port mac colum which is in form"},{"line_number":468,"context_line":"        # [\"\u003cport_mac\u003e \u003cip1\u003e \u003cip2\u003e ... \u003cipN\u003e\"]"},{"line_number":469,"context_line":"        mac_field_attrs \u003d port.mac[0].split()"},{"line_number":470,"context_line":"        if len(mac_field_attrs) \u003e 1:"}],"source_content_type":"text/x-python","patch_set":5,"id":"4d41d1ba_740366f7","line":467,"range":{"start_line":467,"start_character":41,"end_line":467,"end_character":46},"updated":"2022-12-15 18:52:30.000000000","message":"nit: column","commit_id":"44aca3ceff4b57dc3e78addcd8db1a90e1ac5dce"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"f7704c89c4585a47ab928387673a3d618b22341b","unresolved":true,"context_lines":[{"line_number":464,"context_line":"        iptables_mgr.apply()"},{"line_number":465,"context_line":""},{"line_number":466,"context_line":"    def _get_port_ips(self, port):"},{"line_number":467,"context_line":"        # Retreive IPs from the port mac colum which is in form"},{"line_number":468,"context_line":"        # [\"\u003cport_mac\u003e \u003cip1\u003e \u003cip2\u003e ... \u003cipN\u003e\"]"},{"line_number":469,"context_line":"        mac_field_attrs \u003d port.mac[0].split()"},{"line_number":470,"context_line":"        if len(mac_field_attrs) \u003e 1:"}],"source_content_type":"text/x-python","patch_set":5,"id":"513c3b1f_d6a9e5b6","line":467,"range":{"start_line":467,"start_character":10,"end_line":467,"end_character":18},"updated":"2022-12-15 18:52:30.000000000","message":"nit: retrieve","commit_id":"44aca3ceff4b57dc3e78addcd8db1a90e1ac5dce"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"960e6ab46b32259b29db41f7a75cc855c03ad91b","unresolved":false,"context_lines":[{"line_number":464,"context_line":"        iptables_mgr.apply()"},{"line_number":465,"context_line":""},{"line_number":466,"context_line":"    def _get_port_ips(self, port):"},{"line_number":467,"context_line":"        # Retreive IPs from the port mac colum which is in form"},{"line_number":468,"context_line":"        # [\"\u003cport_mac\u003e \u003cip1\u003e \u003cip2\u003e ... \u003cipN\u003e\"]"},{"line_number":469,"context_line":"        mac_field_attrs \u003d port.mac[0].split()"},{"line_number":470,"context_line":"        if len(mac_field_attrs) \u003e 1:"}],"source_content_type":"text/x-python","patch_set":5,"id":"790435c6_94148769","line":467,"range":{"start_line":467,"start_character":41,"end_line":467,"end_character":46},"in_reply_to":"4d41d1ba_740366f7","updated":"2022-12-19 14:08:11.000000000","message":"Ack","commit_id":"44aca3ceff4b57dc3e78addcd8db1a90e1ac5dce"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"960e6ab46b32259b29db41f7a75cc855c03ad91b","unresolved":false,"context_lines":[{"line_number":464,"context_line":"        iptables_mgr.apply()"},{"line_number":465,"context_line":""},{"line_number":466,"context_line":"    def _get_port_ips(self, port):"},{"line_number":467,"context_line":"        # Retreive IPs from the port mac colum which is in form"},{"line_number":468,"context_line":"        # [\"\u003cport_mac\u003e \u003cip1\u003e \u003cip2\u003e ... \u003cipN\u003e\"]"},{"line_number":469,"context_line":"        mac_field_attrs \u003d port.mac[0].split()"},{"line_number":470,"context_line":"        if len(mac_field_attrs) \u003e 1:"}],"source_content_type":"text/x-python","patch_set":5,"id":"0049857d_4d0dc003","line":467,"range":{"start_line":467,"start_character":10,"end_line":467,"end_character":18},"in_reply_to":"513c3b1f_d6a9e5b6","updated":"2022-12-19 14:08:11.000000000","message":"Ack","commit_id":"44aca3ceff4b57dc3e78addcd8db1a90e1ac5dce"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"f7704c89c4585a47ab928387673a3d618b22341b","unresolved":true,"context_lines":[{"line_number":468,"context_line":"        # [\"\u003cport_mac\u003e \u003cip1\u003e \u003cip2\u003e ... \u003cipN\u003e\"]"},{"line_number":469,"context_line":"        mac_field_attrs \u003d port.mac[0].split()"},{"line_number":470,"context_line":"        if len(mac_field_attrs) \u003e 1:"},{"line_number":471,"context_line":"            return mac_field_attrs[1:]"},{"line_number":472,"context_line":"        LOG.debug(\"Port %s IP addresses were not retrieved from the \""},{"line_number":473,"context_line":"                  \"Port_Binding MAC column %s\", port.uuid, mac_field_attrs)"},{"line_number":474,"context_line":"        return"}],"source_content_type":"text/x-python","patch_set":5,"id":"8ac443b1_3a1c0cc2","line":471,"updated":"2022-12-15 18:52:30.000000000","message":"slicing is gracious to empty lists, so you can just always return \"attrs[1:]\"","commit_id":"44aca3ceff4b57dc3e78addcd8db1a90e1ac5dce"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"960e6ab46b32259b29db41f7a75cc855c03ad91b","unresolved":false,"context_lines":[{"line_number":468,"context_line":"        # [\"\u003cport_mac\u003e \u003cip1\u003e \u003cip2\u003e ... \u003cipN\u003e\"]"},{"line_number":469,"context_line":"        mac_field_attrs \u003d port.mac[0].split()"},{"line_number":470,"context_line":"        if len(mac_field_attrs) \u003e 1:"},{"line_number":471,"context_line":"            return mac_field_attrs[1:]"},{"line_number":472,"context_line":"        LOG.debug(\"Port %s IP addresses were not retrieved from the \""},{"line_number":473,"context_line":"                  \"Port_Binding MAC column %s\", port.uuid, mac_field_attrs)"},{"line_number":474,"context_line":"        return"}],"source_content_type":"text/x-python","patch_set":5,"id":"668f9e21_0744ac1f","line":471,"in_reply_to":"8ac443b1_3a1c0cc2","updated":"2022-12-19 14:08:11.000000000","message":"Ack","commit_id":"44aca3ceff4b57dc3e78addcd8db1a90e1ac5dce"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"f7704c89c4585a47ab928387673a3d618b22341b","unresolved":true,"context_lines":[{"line_number":478,"context_line":"        for datapath_port_ip in datapath_ports_ips:"},{"line_number":479,"context_line":"            ip_obj \u003d netaddr.IPAddress(datapath_port_ip)"},{"line_number":480,"context_line":"            for metadata_cidr in metadata_port_cidrs:"},{"line_number":481,"context_line":"                if ip_obj in netaddr.IPNetwork(metadata_cidr):"},{"line_number":482,"context_line":"                    active_subnets_cidrs.add(metadata_cidr)"},{"line_number":483,"context_line":"                    break"},{"line_number":484,"context_line":"        return active_subnets_cidrs"}],"source_content_type":"text/x-python","patch_set":5,"id":"e62150f9_5088a93b","line":481,"updated":"2022-12-15 18:52:30.000000000","message":"nit: this line parses metadata_cidr over and over for each of datapath_port_ip. Perhaps consider pre-parsing all of them into IPNetwork objects once, then iterate over the list of IPNetworks.","commit_id":"44aca3ceff4b57dc3e78addcd8db1a90e1ac5dce"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"960e6ab46b32259b29db41f7a75cc855c03ad91b","unresolved":false,"context_lines":[{"line_number":478,"context_line":"        for datapath_port_ip in datapath_ports_ips:"},{"line_number":479,"context_line":"            ip_obj \u003d netaddr.IPAddress(datapath_port_ip)"},{"line_number":480,"context_line":"            for metadata_cidr in metadata_port_cidrs:"},{"line_number":481,"context_line":"                if ip_obj in netaddr.IPNetwork(metadata_cidr):"},{"line_number":482,"context_line":"                    active_subnets_cidrs.add(metadata_cidr)"},{"line_number":483,"context_line":"                    break"},{"line_number":484,"context_line":"        return active_subnets_cidrs"}],"source_content_type":"text/x-python","patch_set":5,"id":"415f9029_8bc3d54d","line":481,"in_reply_to":"e62150f9_5088a93b","updated":"2022-12-19 14:08:11.000000000","message":"Done","commit_id":"44aca3ceff4b57dc3e78addcd8db1a90e1ac5dce"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"f7704c89c4585a47ab928387673a3d618b22341b","unresolved":true,"context_lines":[{"line_number":550,"context_line":"        for chassis_port in self._vif_ports(chassis_ports):"},{"line_number":551,"context_line":"            if str(chassis_port.datapath.uuid) \u003d\u003d datapath_uuid:"},{"line_number":552,"context_line":"                port_ips \u003d self._get_port_ips(chassis_port)"},{"line_number":553,"context_line":"                if port_ips:"},{"line_number":554,"context_line":"                    datapath_ports_ips.extend(port_ips)"},{"line_number":555,"context_line":""},{"line_number":556,"context_line":"        if not datapath_ports_ips:"}],"source_content_type":"text/x-python","patch_set":5,"id":"b7ec5725_2e42bb44","line":553,"updated":"2022-12-15 18:52:30.000000000","message":"if you make _get_port_ips always return a list (and not None), then you could remove this \u0027if\u0027 check and always .extend()","commit_id":"44aca3ceff4b57dc3e78addcd8db1a90e1ac5dce"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"960e6ab46b32259b29db41f7a75cc855c03ad91b","unresolved":false,"context_lines":[{"line_number":550,"context_line":"        for chassis_port in self._vif_ports(chassis_ports):"},{"line_number":551,"context_line":"            if str(chassis_port.datapath.uuid) \u003d\u003d datapath_uuid:"},{"line_number":552,"context_line":"                port_ips \u003d self._get_port_ips(chassis_port)"},{"line_number":553,"context_line":"                if port_ips:"},{"line_number":554,"context_line":"                    datapath_ports_ips.extend(port_ips)"},{"line_number":555,"context_line":""},{"line_number":556,"context_line":"        if not datapath_ports_ips:"}],"source_content_type":"text/x-python","patch_set":5,"id":"d1ed4624_1e8bd405","line":553,"in_reply_to":"b7ec5725_2e42bb44","updated":"2022-12-19 14:08:11.000000000","message":"Done","commit_id":"44aca3ceff4b57dc3e78addcd8db1a90e1ac5dce"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"f7704c89c4585a47ab928387673a3d618b22341b","unresolved":true,"context_lines":[{"line_number":556,"context_line":"        if not datapath_ports_ips:"},{"line_number":557,"context_line":"            LOG.debug(\"No valid VIF ports were found for network %s, \""},{"line_number":558,"context_line":"                      \"tearing the namespace down if needed\", net_name)"},{"line_number":559,"context_line":"            self.teardown_datapath(net_name)"},{"line_number":560,"context_line":"            return"},{"line_number":561,"context_line":""},{"line_number":562,"context_line":"        return net_name, datapath_ports_ips, metadata_port_info"}],"source_content_type":"text/x-python","patch_set":5,"id":"1285b8bb_9a6098d9","line":559,"updated":"2022-12-15 18:52:30.000000000","message":"(perhaps it\u0027s just me but) I find it a bit confusing that a function called \"_get_provision_params\" also deprovisions datapaths. It seems to me that it would be better to leave the _get_provision_params function to calculate the intended result, and to *act* on the intended result outside of the function (in provision_datapath).","commit_id":"44aca3ceff4b57dc3e78addcd8db1a90e1ac5dce"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"960e6ab46b32259b29db41f7a75cc855c03ad91b","unresolved":false,"context_lines":[{"line_number":556,"context_line":"        if not datapath_ports_ips:"},{"line_number":557,"context_line":"            LOG.debug(\"No valid VIF ports were found for network %s, \""},{"line_number":558,"context_line":"                      \"tearing the namespace down if needed\", net_name)"},{"line_number":559,"context_line":"            self.teardown_datapath(net_name)"},{"line_number":560,"context_line":"            return"},{"line_number":561,"context_line":""},{"line_number":562,"context_line":"        return net_name, datapath_ports_ips, metadata_port_info"}],"source_content_type":"text/x-python","patch_set":5,"id":"d09d26a8_188057b9","line":559,"in_reply_to":"1285b8bb_9a6098d9","updated":"2022-12-19 14:08:11.000000000","message":"Ack","commit_id":"44aca3ceff4b57dc3e78addcd8db1a90e1ac5dce"}]}