)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"ddad4b3bb8cac4f325ab100e05b2d5348cfafca1","unresolved":true,"context_lines":[{"line_number":10,"context_line":"SRC mac flows for the instances of same subnet on that compute node."},{"line_number":11,"context_line":"The instances are not reachable from any other network."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"This patch checks if the DVR router port is gateway for the subnet"},{"line_number":14,"context_line":"or not. And deletes the DVR-SRC mac flows only if it is gateway port."},{"line_number":15,"context_line":"The DVR-SRC mac flows are deleted if the gateway is not set for the subnet."},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"Change-Id: Iadc1671c862f8c01e5761e92b82a04849d4bb411"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":11,"id":"4bfdcee0_d7b536b8","line":14,"range":{"start_line":13,"start_character":18,"end_line":14,"end_character":7},"updated":"2020-11-24 11:28:53.000000000","message":"I wonder check the port\u0027s device_id whether belongs to a router can cover this?\nFrom the reproduce step you added in the LP, when we try to remove the port \"to-n3\" from the \"r3\", the port to-n3\u0027s device_id should be r3\u0027s router_id. So ovs-agent should not touch/remove any thing that belongs to r1. How about we fix the bug in such way?","commit_id":"3180d400593839e492e63c2ac86801a0468ee85e"},{"author":{"_account_id":10366,"name":"Hemanth N","email":"hemanth.nakkina@canonical.com","username":"Hemanth"},"change_message_id":"cf623f1725edc5c19b614855b0d96edab98b577d","unresolved":true,"context_lines":[{"line_number":10,"context_line":"SRC mac flows for the instances of same subnet on that compute node."},{"line_number":11,"context_line":"The instances are not reachable from any other network."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"This patch checks if the DVR router port is gateway for the subnet"},{"line_number":14,"context_line":"or not. And deletes the DVR-SRC mac flows only if it is gateway port."},{"line_number":15,"context_line":"The DVR-SRC mac flows are deleted if the gateway is not set for the subnet."},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"Change-Id: Iadc1671c862f8c01e5761e92b82a04849d4bb411"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":11,"id":"6db94a76_2cc19f47","line":14,"range":{"start_line":13,"start_character":18,"end_line":14,"end_character":7},"in_reply_to":"4bfdcee0_d7b536b8","updated":"2020-11-24 12:48:34.000000000","message":"ovs-agent is deleting the dvr-src mac flows based on subnet (not the router). Here we are ensuring to place an extra check to do delete operation only when the port is gateway port of the subnet.\nIn the example, port to-n3 device-id is r3 but the port belongs to network n1 (or subnet sn1) and all the DVR-SRC mac flows belonging to the subnet (all VM connected to sn1) are deleted.","commit_id":"3180d400593839e492e63c2ac86801a0468ee85e"}],"neutron/plugins/ml2/drivers/openvswitch/agent/ovs_dvr_neutron_agent.py":[{"author":{"_account_id":6737,"name":"Edward Hope-Morley","email":"edward.hope-morley@canonical.com","username":"hopem"},"change_message_id":"c6975a4f7941027396e5eec6b3124297e6c4050a","unresolved":false,"context_lines":[{"line_number":115,"context_line":"        return self.ofport"},{"line_number":116,"context_line":""},{"line_number":117,"context_line":"    def add_ip(self, subnet_id, ip):"},{"line_number":118,"context_line":"        self.ips[subnet_id] \u003d ip"},{"line_number":119,"context_line":""},{"line_number":120,"context_line":"    def get_ip(self, subnet_id):"},{"line_number":121,"context_line":"        return self.ips.get(subnet_id, None)"}],"source_content_type":"text/x-python","patch_set":3,"id":"9f560f44_7ef2299d","line":118,"range":{"start_line":118,"start_character":30,"end_line":118,"end_character":32},"updated":"2020-09-18 11:16:51.000000000","message":"this is saying that you can only have one address configured per subnet but e.g. qg ports on snat ns will have all/any fip addresses for unbound ports. I know that\u0027s a different scenario but wonder if we need to take it into account here.","commit_id":"bf1dc622a26b4cf8cd8d441a6e64b093eb3fdbe0"},{"author":{"_account_id":10366,"name":"Hemanth N","email":"hemanth.nakkina@canonical.com","username":"Hemanth"},"change_message_id":"fd4822624cfac047687c5ccda8080af66606d9c8","unresolved":false,"context_lines":[{"line_number":115,"context_line":"        return self.ofport"},{"line_number":116,"context_line":""},{"line_number":117,"context_line":"    def add_ip(self, subnet_id, ip):"},{"line_number":118,"context_line":"        self.ips[subnet_id] \u003d ip"},{"line_number":119,"context_line":""},{"line_number":120,"context_line":"    def get_ip(self, subnet_id):"},{"line_number":121,"context_line":"        return self.ips.get(subnet_id, None)"}],"source_content_type":"text/x-python","patch_set":3,"id":"9f560f44_6ddb6f7c","line":118,"range":{"start_line":118,"start_character":30,"end_line":118,"end_character":32},"in_reply_to":"9f560f44_7ef2299d","updated":"2020-09-22 11:23:40.000000000","message":"Done","commit_id":"bf1dc622a26b4cf8cd8d441a6e64b093eb3fdbe0"},{"author":{"_account_id":6737,"name":"Edward Hope-Morley","email":"edward.hope-morley@canonical.com","username":"hopem"},"change_message_id":"c6975a4f7941027396e5eec6b3124297e6c4050a","unresolved":false,"context_lines":[{"line_number":665,"context_line":""},{"line_number":666,"context_line":"            fixed_ip \u003d ovsport.get_ip(sub_uuid)"},{"line_number":667,"context_line":"            is_dvr_gateway_port \u003d False"},{"line_number":668,"context_line":"            if subnet_info[\u0027gateway_ip\u0027] and fixed_ip:"},{"line_number":669,"context_line":"                if fixed_ip \u003d\u003d subnet_info[\u0027gateway_ip\u0027]:"},{"line_number":670,"context_line":"                    is_dvr_gateway_port \u003d True"},{"line_number":671,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"9f560f44_1ec5ed0a","line":668,"range":{"start_line":668,"start_character":53,"end_line":668,"end_character":54},"updated":"2020-09-18 11:16:51.000000000","message":"Since you can create a router with subnets attached while external-gateway is unset, perhaps we need to allow for this continue in that case too i.e. where subnet_info[\u0027gateway_ip\u0027] is None and maybe for the same reason we need to do subnet_info.get(\u0027gateway_ip\u0027). I\u0027d have to look closer at surrounding code to be sure.","commit_id":"bf1dc622a26b4cf8cd8d441a6e64b093eb3fdbe0"},{"author":{"_account_id":10366,"name":"Hemanth N","email":"hemanth.nakkina@canonical.com","username":"Hemanth"},"change_message_id":"fd4822624cfac047687c5ccda8080af66606d9c8","unresolved":false,"context_lines":[{"line_number":665,"context_line":""},{"line_number":666,"context_line":"            fixed_ip \u003d ovsport.get_ip(sub_uuid)"},{"line_number":667,"context_line":"            is_dvr_gateway_port \u003d False"},{"line_number":668,"context_line":"            if subnet_info[\u0027gateway_ip\u0027] and fixed_ip:"},{"line_number":669,"context_line":"                if fixed_ip \u003d\u003d subnet_info[\u0027gateway_ip\u0027]:"},{"line_number":670,"context_line":"                    is_dvr_gateway_port \u003d True"},{"line_number":671,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"9f560f44_4dd82b7e","line":668,"range":{"start_line":668,"start_character":53,"end_line":668,"end_character":54},"in_reply_to":"9f560f44_1ec5ed0a","updated":"2020-09-22 11:23:40.000000000","message":"Done","commit_id":"bf1dc622a26b4cf8cd8d441a6e64b093eb3fdbe0"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"9384a22b2a925292b8602250dfebb105e3d50cba","unresolved":false,"context_lines":[{"line_number":680,"context_line":"            # remove vm dvr src mac rules only if the ovsport"},{"line_number":681,"context_line":"            # is gateway for the subnet or if the gateway is"},{"line_number":682,"context_line":"            # not set on the subnet"},{"line_number":683,"context_line":"            if is_dvr_gateway_port or is_subnet_gateway_unset:"},{"line_number":684,"context_line":"                # DVR is no more owner"},{"line_number":685,"context_line":"                ldm.set_dvr_owned(False)"},{"line_number":686,"context_line":"                # remove all vm rules for this dvr subnet"}],"source_content_type":"text/x-python","patch_set":6,"id":"9f560f44_6e958932","line":683,"updated":"2020-10-01 07:53:15.000000000","message":"Using the same reproducer method you submitted [1], when you remove all router ports assigned to a network, the dvr_to_src_mac flows are not deleted.\n\n[1]https://bugs.launchpad.net/neutron/+bug/1892405/comments/4","commit_id":"a2137fde85365024e5d20ee48da275e01afba9f7"},{"author":{"_account_id":10366,"name":"Hemanth N","email":"hemanth.nakkina@canonical.com","username":"Hemanth"},"change_message_id":"e6a15748f12e0c5abecd51e9d1885ac8282d79ab","unresolved":false,"context_lines":[{"line_number":680,"context_line":"            # remove vm dvr src mac rules only if the ovsport"},{"line_number":681,"context_line":"            # is gateway for the subnet or if the gateway is"},{"line_number":682,"context_line":"            # not set on the subnet"},{"line_number":683,"context_line":"            if is_dvr_gateway_port or is_subnet_gateway_unset:"},{"line_number":684,"context_line":"                # DVR is no more owner"},{"line_number":685,"context_line":"                ldm.set_dvr_owned(False)"},{"line_number":686,"context_line":"                # remove all vm rules for this dvr subnet"}],"source_content_type":"text/x-python","patch_set":6,"id":"9f560f44_daef2baf","line":683,"in_reply_to":"9f560f44_6e958932","updated":"2020-10-01 12:19:13.000000000","message":"Thanks for catching the issue. Fixed in PS7","commit_id":"a2137fde85365024e5d20ee48da275e01afba9f7"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"e39ef8f44963e55c0569ea555e5cb16ed6b7de35","unresolved":false,"context_lines":[{"line_number":98,"context_line":""},{"line_number":99,"context_line":"    def __str__(self):"},{"line_number":100,"context_line":"        return (\"OVSPort: id \u003d %s, ofport \u003d %s, mac \u003d %s, \""},{"line_number":101,"context_line":"                \"device_owner \u003d %s, subnets \u003d %s\" %"},{"line_number":102,"context_line":"                (self.id, self.ofport, self.mac,"},{"line_number":103,"context_line":"                 self.device_owner, self.subnets))"},{"line_number":104,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"9f560f44_8d7b75f2","line":101,"updated":"2020-11-09 21:28:19.000000000","message":"maybe You can add this new attribute \"ips\" here too?","commit_id":"554f9e5e4a7c7a0f22ce5d36a66fbaf98bd0a6f1"},{"author":{"_account_id":6737,"name":"Edward Hope-Morley","email":"edward.hope-morley@canonical.com","username":"hopem"},"change_message_id":"93a126cf4751a3b934708bbc33897fe5c7af69a2","unresolved":false,"context_lines":[{"line_number":106,"context_line":"        self.subnets.add(subnet_id)"},{"line_number":107,"context_line":""},{"line_number":108,"context_line":"    def remove_subnet(self, subnet_id):"},{"line_number":109,"context_line":"        self.subnets.remove(subnet_id)"},{"line_number":110,"context_line":"        self.ips.pop(subnet_id, None)"},{"line_number":111,"context_line":""},{"line_number":112,"context_line":"    def remove_all_subnets(self):"}],"source_content_type":"text/x-python","patch_set":8,"id":"3f65232a_a40dfc3f","line":109,"range":{"start_line":109,"start_character":0,"end_line":109,"end_character":38},"updated":"2020-10-24 15:01:13.000000000","message":"This is a cosmetic observation but:\n\nsince add_subnet is called from multiple places i.e.\n\ndef _bind_distributed_router_interface_port()\ndef _bind_port_on_dvr_subnet()\ndef _bind_centralized_snat_port_on_dvr_subnet()\n\nand not all of them will subsequently call add_ip(), mightn\u0027t it better to incorporate add_ip() into add_subnet() by having an extra optional arg e.g.\n\ndef add_subnet(self, subnet_id, fixed_ip\u003dNone):\n   ...\n\nto me that would be more inline with the logic of having the ip removal part of remove_subnet()","commit_id":"554f9e5e4a7c7a0f22ce5d36a66fbaf98bd0a6f1"},{"author":{"_account_id":10366,"name":"Hemanth N","email":"hemanth.nakkina@canonical.com","username":"Hemanth"},"change_message_id":"fb75ed7a10ffd0f00a73df558e5ee1f27883aa96","unresolved":false,"context_lines":[{"line_number":106,"context_line":"        self.subnets.add(subnet_id)"},{"line_number":107,"context_line":""},{"line_number":108,"context_line":"    def remove_subnet(self, subnet_id):"},{"line_number":109,"context_line":"        self.subnets.remove(subnet_id)"},{"line_number":110,"context_line":"        self.ips.pop(subnet_id, None)"},{"line_number":111,"context_line":""},{"line_number":112,"context_line":"    def remove_all_subnets(self):"}],"source_content_type":"text/x-python","patch_set":8,"id":"1f621f24_c5ca8f6c","line":109,"range":{"start_line":109,"start_character":0,"end_line":109,"end_character":38},"in_reply_to":"3f65232a_a40dfc3f","updated":"2020-11-04 06:05:45.000000000","message":"Done","commit_id":"554f9e5e4a7c7a0f22ce5d36a66fbaf98bd0a6f1"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"e39ef8f44963e55c0569ea555e5cb16ed6b7de35","unresolved":false,"context_lines":[{"line_number":126,"context_line":"        return self.ofport"},{"line_number":127,"context_line":""},{"line_number":128,"context_line":"    def add_ip(self, subnet_id, ip):"},{"line_number":129,"context_line":"        if subnet_id in self.ips:"},{"line_number":130,"context_line":"            self.ips[subnet_id].append(ip)"},{"line_number":131,"context_line":"        else:"},{"line_number":132,"context_line":"            self.ips[subnet_id] \u003d [ip]"}],"source_content_type":"text/x-python","patch_set":8,"id":"9f560f44_6d70a10a","line":129,"updated":"2020-11-09 21:28:19.000000000","message":"if You would use collections.defaultdict(list) as self.ips then You wouldn\u0027t need this \"if\" here","commit_id":"554f9e5e4a7c7a0f22ce5d36a66fbaf98bd0a6f1"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"e39ef8f44963e55c0569ea555e5cb16ed6b7de35","unresolved":false,"context_lines":[{"line_number":132,"context_line":"            self.ips[subnet_id] \u003d [ip]"},{"line_number":133,"context_line":""},{"line_number":134,"context_line":"    def get_ip(self, subnet_id):"},{"line_number":135,"context_line":"        return self.ips.get(subnet_id, None)"},{"line_number":136,"context_line":""},{"line_number":137,"context_line":""},{"line_number":138,"context_line":"@profiler.trace_cls(\"ovs_dvr_agent\")"}],"source_content_type":"text/x-python","patch_set":8,"id":"9f560f44_6d49c1b6","line":135,"range":{"start_line":135,"start_character":39,"end_line":135,"end_character":43},"updated":"2020-11-09 21:28:19.000000000","message":"nitty nit: this \"None\" isn\u0027t necessary here","commit_id":"554f9e5e4a7c7a0f22ce5d36a66fbaf98bd0a6f1"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"e39ef8f44963e55c0569ea555e5cb16ed6b7de35","unresolved":false,"context_lines":[{"line_number":94,"context_line":"        self.subnets \u003d set()"},{"line_number":95,"context_line":"        self.device_owner \u003d device_owner"},{"line_number":96,"context_line":"        # Currently, this is updated only for DVR router interfaces"},{"line_number":97,"context_line":"        self.ips \u003d {}"},{"line_number":98,"context_line":""},{"line_number":99,"context_line":"    def __str__(self):"},{"line_number":100,"context_line":"        return (\"OVSPort: id \u003d %s, ofport \u003d %s, mac \u003d %s, \""}],"source_content_type":"text/x-python","patch_set":9,"id":"1f621f24_540c3393","line":97,"updated":"2020-11-09 21:28:19.000000000","message":"if You would use collections.defaultdict(list) here, You wouldn\u0027t need \"if..else..\" in L110-113","commit_id":"a7385fde3a9b6c5b8de70899c10472a68f2eeaf5"},{"author":{"_account_id":10366,"name":"Hemanth N","email":"hemanth.nakkina@canonical.com","username":"Hemanth"},"change_message_id":"c3d45630e5a6c83de86ef59233ae2e9d3333f048","unresolved":false,"context_lines":[{"line_number":94,"context_line":"        self.subnets \u003d set()"},{"line_number":95,"context_line":"        self.device_owner \u003d device_owner"},{"line_number":96,"context_line":"        # Currently, this is updated only for DVR router interfaces"},{"line_number":97,"context_line":"        self.ips \u003d {}"},{"line_number":98,"context_line":""},{"line_number":99,"context_line":"    def __str__(self):"},{"line_number":100,"context_line":"        return (\"OVSPort: id \u003d %s, ofport \u003d %s, mac \u003d %s, \""}],"source_content_type":"text/x-python","patch_set":9,"id":"ff46ee8b_c41409b0","line":97,"in_reply_to":"1f621f24_540c3393","updated":"2020-11-24 06:38:21.000000000","message":"Fixed in patchset 11","commit_id":"a7385fde3a9b6c5b8de70899c10472a68f2eeaf5"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"e39ef8f44963e55c0569ea555e5cb16ed6b7de35","unresolved":false,"context_lines":[{"line_number":100,"context_line":"        return (\"OVSPort: id \u003d %s, ofport \u003d %s, mac \u003d %s, \""},{"line_number":101,"context_line":"                \"device_owner \u003d %s, subnets \u003d %s\" %"},{"line_number":102,"context_line":"                (self.id, self.ofport, self.mac,"},{"line_number":103,"context_line":"                 self.device_owner, self.subnets))"},{"line_number":104,"context_line":""},{"line_number":105,"context_line":"    def add_subnet(self, subnet_id, fixed_ip\u003dNone):"},{"line_number":106,"context_line":"        self.subnets.add(subnet_id)"}],"source_content_type":"text/x-python","patch_set":9,"id":"1f621f24_3438973b","line":103,"updated":"2020-11-09 21:28:19.000000000","message":"nit: You could add self.ips to be printed here also","commit_id":"a7385fde3a9b6c5b8de70899c10472a68f2eeaf5"},{"author":{"_account_id":10366,"name":"Hemanth N","email":"hemanth.nakkina@canonical.com","username":"Hemanth"},"change_message_id":"c3d45630e5a6c83de86ef59233ae2e9d3333f048","unresolved":false,"context_lines":[{"line_number":100,"context_line":"        return (\"OVSPort: id \u003d %s, ofport \u003d %s, mac \u003d %s, \""},{"line_number":101,"context_line":"                \"device_owner \u003d %s, subnets \u003d %s\" %"},{"line_number":102,"context_line":"                (self.id, self.ofport, self.mac,"},{"line_number":103,"context_line":"                 self.device_owner, self.subnets))"},{"line_number":104,"context_line":""},{"line_number":105,"context_line":"    def add_subnet(self, subnet_id, fixed_ip\u003dNone):"},{"line_number":106,"context_line":"        self.subnets.add(subnet_id)"}],"source_content_type":"text/x-python","patch_set":9,"id":"4c73a8b4_b6baea38","line":103,"in_reply_to":"1f621f24_3438973b","updated":"2020-11-24 06:38:21.000000000","message":"Fixed in patchset 11","commit_id":"a7385fde3a9b6c5b8de70899c10472a68f2eeaf5"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"e39ef8f44963e55c0569ea555e5cb16ed6b7de35","unresolved":false,"context_lines":[{"line_number":133,"context_line":"        return self.ofport"},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"    def get_ip(self, subnet_id):"},{"line_number":136,"context_line":"        return self.ips.get(subnet_id, None)"},{"line_number":137,"context_line":""},{"line_number":138,"context_line":""},{"line_number":139,"context_line":"@profiler.trace_cls(\"ovs_dvr_agent\")"}],"source_content_type":"text/x-python","patch_set":9,"id":"1f621f24_9429abe7","line":136,"range":{"start_line":136,"start_character":39,"end_line":136,"end_character":43},"updated":"2020-11-09 21:28:19.000000000","message":"nit: this \"None\" is not needed here. It\u0027s default","commit_id":"a7385fde3a9b6c5b8de70899c10472a68f2eeaf5"},{"author":{"_account_id":10366,"name":"Hemanth N","email":"hemanth.nakkina@canonical.com","username":"Hemanth"},"change_message_id":"c3d45630e5a6c83de86ef59233ae2e9d3333f048","unresolved":false,"context_lines":[{"line_number":133,"context_line":"        return self.ofport"},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"    def get_ip(self, subnet_id):"},{"line_number":136,"context_line":"        return self.ips.get(subnet_id, None)"},{"line_number":137,"context_line":""},{"line_number":138,"context_line":""},{"line_number":139,"context_line":"@profiler.trace_cls(\"ovs_dvr_agent\")"}],"source_content_type":"text/x-python","patch_set":9,"id":"92931fef_51e70e21","line":136,"range":{"start_line":136,"start_character":39,"end_line":136,"end_character":43},"in_reply_to":"1f621f24_9429abe7","updated":"2020-11-24 06:38:21.000000000","message":"Fixed in patchset 11","commit_id":"a7385fde3a9b6c5b8de70899c10472a68f2eeaf5"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"e39ef8f44963e55c0569ea555e5cb16ed6b7de35","unresolved":false,"context_lines":[{"line_number":680,"context_line":""},{"line_number":681,"context_line":"            fixed_ip \u003d ovsport.get_ip(sub_uuid)"},{"line_number":682,"context_line":"            is_dvr_gateway_port \u003d False"},{"line_number":683,"context_line":"            is_subnet_gateway_unset \u003d False"},{"line_number":684,"context_line":"            if subnet_info[\u0027gateway_ip\u0027] is None:"},{"line_number":685,"context_line":"                is_subnet_gateway_unset \u003d True"},{"line_number":686,"context_line":"            if subnet_info[\u0027gateway_ip\u0027] and fixed_ip:"}],"source_content_type":"text/x-python","patch_set":9,"id":"1f621f24_f468df1d","line":683,"updated":"2020-11-09 21:28:19.000000000","message":"nit: it could be simply:\n\n    is_subnet_gateway_unset \u003d subnet_info[\u0027gateway_ip\u0027] is None","commit_id":"a7385fde3a9b6c5b8de70899c10472a68f2eeaf5"},{"author":{"_account_id":10366,"name":"Hemanth N","email":"hemanth.nakkina@canonical.com","username":"Hemanth"},"change_message_id":"c3d45630e5a6c83de86ef59233ae2e9d3333f048","unresolved":false,"context_lines":[{"line_number":680,"context_line":""},{"line_number":681,"context_line":"            fixed_ip \u003d ovsport.get_ip(sub_uuid)"},{"line_number":682,"context_line":"            is_dvr_gateway_port \u003d False"},{"line_number":683,"context_line":"            is_subnet_gateway_unset \u003d False"},{"line_number":684,"context_line":"            if subnet_info[\u0027gateway_ip\u0027] is None:"},{"line_number":685,"context_line":"                is_subnet_gateway_unset \u003d True"},{"line_number":686,"context_line":"            if subnet_info[\u0027gateway_ip\u0027] and fixed_ip:"}],"source_content_type":"text/x-python","patch_set":9,"id":"d87d95b8_2fa4d631","line":683,"in_reply_to":"1f621f24_f468df1d","updated":"2020-11-24 06:38:21.000000000","message":"Fixed in patchset 11","commit_id":"a7385fde3a9b6c5b8de70899c10472a68f2eeaf5"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"e39ef8f44963e55c0569ea555e5cb16ed6b7de35","unresolved":false,"context_lines":[{"line_number":683,"context_line":"            is_subnet_gateway_unset \u003d False"},{"line_number":684,"context_line":"            if subnet_info[\u0027gateway_ip\u0027] is None:"},{"line_number":685,"context_line":"                is_subnet_gateway_unset \u003d True"},{"line_number":686,"context_line":"            if subnet_info[\u0027gateway_ip\u0027] and fixed_ip:"},{"line_number":687,"context_line":"                # since distributed router port must have only one fixed"},{"line_number":688,"context_line":"                # IP, directly use fixed_ips[0]"},{"line_number":689,"context_line":"                if fixed_ip[0] \u003d\u003d subnet_info[\u0027gateway_ip\u0027]:"}],"source_content_type":"text/x-python","patch_set":9,"id":"1f621f24_7454cf53","line":686,"updated":"2020-11-09 21:28:19.000000000","message":"can\u0027t here be simply:\n\n    elif fixed_ip:\n        ....\n\n?","commit_id":"a7385fde3a9b6c5b8de70899c10472a68f2eeaf5"},{"author":{"_account_id":10366,"name":"Hemanth N","email":"hemanth.nakkina@canonical.com","username":"Hemanth"},"change_message_id":"c3d45630e5a6c83de86ef59233ae2e9d3333f048","unresolved":false,"context_lines":[{"line_number":683,"context_line":"            is_subnet_gateway_unset \u003d False"},{"line_number":684,"context_line":"            if subnet_info[\u0027gateway_ip\u0027] is None:"},{"line_number":685,"context_line":"                is_subnet_gateway_unset \u003d True"},{"line_number":686,"context_line":"            if subnet_info[\u0027gateway_ip\u0027] and fixed_ip:"},{"line_number":687,"context_line":"                # since distributed router port must have only one fixed"},{"line_number":688,"context_line":"                # IP, directly use fixed_ips[0]"},{"line_number":689,"context_line":"                if fixed_ip[0] \u003d\u003d subnet_info[\u0027gateway_ip\u0027]:"}],"source_content_type":"text/x-python","patch_set":9,"id":"3d729233_e34549d9","line":686,"in_reply_to":"1f621f24_7454cf53","updated":"2020-11-24 06:38:21.000000000","message":"Rectified because of previous comment change","commit_id":"a7385fde3a9b6c5b8de70899c10472a68f2eeaf5"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"e39ef8f44963e55c0569ea555e5cb16ed6b7de35","unresolved":false,"context_lines":[{"line_number":685,"context_line":"                is_subnet_gateway_unset \u003d True"},{"line_number":686,"context_line":"            if subnet_info[\u0027gateway_ip\u0027] and fixed_ip:"},{"line_number":687,"context_line":"                # since distributed router port must have only one fixed"},{"line_number":688,"context_line":"                # IP, directly use fixed_ips[0]"},{"line_number":689,"context_line":"                if fixed_ip[0] \u003d\u003d subnet_info[\u0027gateway_ip\u0027]:"},{"line_number":690,"context_line":"                    is_dvr_gateway_port \u003d True"},{"line_number":691,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"1f621f24_946ecb2c","line":688,"updated":"2020-11-09 21:28:19.000000000","message":"is this comment correct here? I don\u0027t see fixed_ips anywhere","commit_id":"a7385fde3a9b6c5b8de70899c10472a68f2eeaf5"},{"author":{"_account_id":10366,"name":"Hemanth N","email":"hemanth.nakkina@canonical.com","username":"Hemanth"},"change_message_id":"c3d45630e5a6c83de86ef59233ae2e9d3333f048","unresolved":false,"context_lines":[{"line_number":685,"context_line":"                is_subnet_gateway_unset \u003d True"},{"line_number":686,"context_line":"            if subnet_info[\u0027gateway_ip\u0027] and fixed_ip:"},{"line_number":687,"context_line":"                # since distributed router port must have only one fixed"},{"line_number":688,"context_line":"                # IP, directly use fixed_ips[0]"},{"line_number":689,"context_line":"                if fixed_ip[0] \u003d\u003d subnet_info[\u0027gateway_ip\u0027]:"},{"line_number":690,"context_line":"                    is_dvr_gateway_port \u003d True"},{"line_number":691,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"ddb56f6f_835b781d","line":688,"in_reply_to":"1f621f24_946ecb2c","updated":"2020-11-24 06:38:21.000000000","message":"Fixed in patchset 11","commit_id":"a7385fde3a9b6c5b8de70899c10472a68f2eeaf5"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"ddad4b3bb8cac4f325ab100e05b2d5348cfafca1","unresolved":true,"context_lines":[{"line_number":679,"context_line":""},{"line_number":680,"context_line":"            fixed_ip \u003d ovsport.get_ip(sub_uuid)"},{"line_number":681,"context_line":"            is_dvr_gateway_port \u003d False"},{"line_number":682,"context_line":"            is_subnet_gateway_unset \u003d subnet_info[\u0027gateway_ip\u0027] is None"},{"line_number":683,"context_line":"            if subnet_info[\u0027gateway_ip\u0027] and fixed_ip:"},{"line_number":684,"context_line":"                # since distributed router port must have only one fixed"},{"line_number":685,"context_line":"                # IP, directly use fixed_ip[0]"}],"source_content_type":"text/x-python","patch_set":11,"id":"c9ae2f49_84c15975","line":682,"range":{"start_line":682,"start_character":49,"end_line":682,"end_character":63},"updated":"2020-11-24 11:28:53.000000000","message":"It\u0027s better to use subnet_info.get(\u0027gateway_ip\u0027) to aviod key_error when there is no such key in the dict.","commit_id":"3180d400593839e492e63c2ac86801a0468ee85e"},{"author":{"_account_id":10366,"name":"Hemanth N","email":"hemanth.nakkina@canonical.com","username":"Hemanth"},"change_message_id":"cf623f1725edc5c19b614855b0d96edab98b577d","unresolved":false,"context_lines":[{"line_number":679,"context_line":""},{"line_number":680,"context_line":"            fixed_ip \u003d ovsport.get_ip(sub_uuid)"},{"line_number":681,"context_line":"            is_dvr_gateway_port \u003d False"},{"line_number":682,"context_line":"            is_subnet_gateway_unset \u003d subnet_info[\u0027gateway_ip\u0027] is None"},{"line_number":683,"context_line":"            if subnet_info[\u0027gateway_ip\u0027] and fixed_ip:"},{"line_number":684,"context_line":"                # since distributed router port must have only one fixed"},{"line_number":685,"context_line":"                # IP, directly use fixed_ip[0]"}],"source_content_type":"text/x-python","patch_set":11,"id":"fd2bd89f_8ffd1006","line":682,"range":{"start_line":682,"start_character":49,"end_line":682,"end_character":63},"in_reply_to":"c9ae2f49_84c15975","updated":"2020-11-24 12:48:34.000000000","message":"Done","commit_id":"3180d400593839e492e63c2ac86801a0468ee85e"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"ddad4b3bb8cac4f325ab100e05b2d5348cfafca1","unresolved":true,"context_lines":[{"line_number":680,"context_line":"            fixed_ip \u003d ovsport.get_ip(sub_uuid)"},{"line_number":681,"context_line":"            is_dvr_gateway_port \u003d False"},{"line_number":682,"context_line":"            is_subnet_gateway_unset \u003d subnet_info[\u0027gateway_ip\u0027] is None"},{"line_number":683,"context_line":"            if subnet_info[\u0027gateway_ip\u0027] and fixed_ip:"},{"line_number":684,"context_line":"                # since distributed router port must have only one fixed"},{"line_number":685,"context_line":"                # IP, directly use fixed_ip[0]"},{"line_number":686,"context_line":"                if fixed_ip[0] \u003d\u003d subnet_info[\u0027gateway_ip\u0027]:"}],"source_content_type":"text/x-python","patch_set":11,"id":"e28c21d6_93f1a76c","line":683,"range":{"start_line":683,"start_character":15,"end_line":683,"end_character":40},"updated":"2020-11-24 11:28:53.000000000","message":"Since you have same action upper, so you can directly use \"not is_subnet_gateway_unset\".","commit_id":"3180d400593839e492e63c2ac86801a0468ee85e"},{"author":{"_account_id":10366,"name":"Hemanth N","email":"hemanth.nakkina@canonical.com","username":"Hemanth"},"change_message_id":"cf623f1725edc5c19b614855b0d96edab98b577d","unresolved":false,"context_lines":[{"line_number":680,"context_line":"            fixed_ip \u003d ovsport.get_ip(sub_uuid)"},{"line_number":681,"context_line":"            is_dvr_gateway_port \u003d False"},{"line_number":682,"context_line":"            is_subnet_gateway_unset \u003d subnet_info[\u0027gateway_ip\u0027] is None"},{"line_number":683,"context_line":"            if subnet_info[\u0027gateway_ip\u0027] and fixed_ip:"},{"line_number":684,"context_line":"                # since distributed router port must have only one fixed"},{"line_number":685,"context_line":"                # IP, directly use fixed_ip[0]"},{"line_number":686,"context_line":"                if fixed_ip[0] \u003d\u003d subnet_info[\u0027gateway_ip\u0027]:"}],"source_content_type":"text/x-python","patch_set":11,"id":"e7290fa6_9f3e36bf","line":683,"range":{"start_line":683,"start_character":15,"end_line":683,"end_character":40},"in_reply_to":"e28c21d6_93f1a76c","updated":"2020-11-24 12:48:34.000000000","message":"Done","commit_id":"3180d400593839e492e63c2ac86801a0468ee85e"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"3d2ca7155dc7dc3e121fc724af750e20bc490e30","unresolved":true,"context_lines":[{"line_number":677,"context_line":"            subnet_info \u003d ldm.get_subnet_info()"},{"line_number":678,"context_line":"            ip_version \u003d subnet_info[\u0027ip_version\u0027]"},{"line_number":679,"context_line":""},{"line_number":680,"context_line":"            fixed_ip \u003d ovsport.get_ip(sub_uuid)"},{"line_number":681,"context_line":"            is_dvr_gateway_port \u003d False"},{"line_number":682,"context_line":"            is_subnet_gateway_unset \u003d subnet_info.get(\u0027gateway_ip\u0027) is None"},{"line_number":683,"context_line":"            if not is_subnet_gateway_unset and fixed_ip:"}],"source_content_type":"text/x-python","patch_set":13,"id":"cefc2277_4f19c5d8","line":680,"range":{"start_line":680,"start_character":12,"end_line":680,"end_character":47},"updated":"2020-12-15 17:15:26.000000000","message":"As you stated in L684, there should be only one port per network as router interface port; actually the API do not allow to create two ports on the same subnet. Why the subnet IP is stored as a list?","commit_id":"0baa4e0d2d18cfc454a8cf3e74733559fb67a6da"},{"author":{"_account_id":10366,"name":"Hemanth N","email":"hemanth.nakkina@canonical.com","username":"Hemanth"},"change_message_id":"d43762c0d0e287b421b8afe3f19ef6c1e047c997","unresolved":true,"context_lines":[{"line_number":677,"context_line":"            subnet_info \u003d ldm.get_subnet_info()"},{"line_number":678,"context_line":"            ip_version \u003d subnet_info[\u0027ip_version\u0027]"},{"line_number":679,"context_line":""},{"line_number":680,"context_line":"            fixed_ip \u003d ovsport.get_ip(sub_uuid)"},{"line_number":681,"context_line":"            is_dvr_gateway_port \u003d False"},{"line_number":682,"context_line":"            is_subnet_gateway_unset \u003d subnet_info.get(\u0027gateway_ip\u0027) is None"},{"line_number":683,"context_line":"            if not is_subnet_gateway_unset and fixed_ip:"}],"source_content_type":"text/x-python","patch_set":13,"id":"ef4a223b_88b089d1","line":680,"range":{"start_line":680,"start_character":12,"end_line":680,"end_character":47},"in_reply_to":"cefc2277_4f19c5d8","updated":"2020-12-16 06:00:40.000000000","message":"subnet ip is modified as list based on review comments [1]\n\n[1] https://review.opendev.org/c/openstack/neutron/+/752248/3/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_dvr_neutron_agent.py#118","commit_id":"0baa4e0d2d18cfc454a8cf3e74733559fb67a6da"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"db95ae53b8edf5657c235a0deaeacfc7eede1f1d","unresolved":true,"context_lines":[{"line_number":679,"context_line":""},{"line_number":680,"context_line":"            fixed_ip \u003d ovsport.get_ip(sub_uuid)"},{"line_number":681,"context_line":"            is_dvr_gateway_port \u003d False"},{"line_number":682,"context_line":"            is_subnet_gateway_unset \u003d subnet_info.get(\u0027gateway_ip\u0027) is None"},{"line_number":683,"context_line":"            if not is_subnet_gateway_unset and fixed_ip:"},{"line_number":684,"context_line":"                # since distributed router port must have only one fixed"},{"line_number":685,"context_line":"                # IP, directly use fixed_ip[0]"}],"source_content_type":"text/x-python","patch_set":13,"id":"73abd4a8_f425eb9a","line":682,"range":{"start_line":682,"start_character":12,"end_line":682,"end_character":35},"updated":"2020-12-15 15:31:19.000000000","message":"nit: I\u0027m not a fan of negatives since then we use double-negatives below (\"not unset\") but the code looks correct.  Just pulling out the gateway IP would have been better since it\u0027s used on L686.\n\n    subnet_gateway \u003d subnet_info.get(\u0027gateway_ip\u0027)\n    if fixed_ip and fixed_ip[0] \u003d\u003d subnet_gateway:\n        is_dvr_gateway_port \u003d True\n\nOf course it\u0027s untested...","commit_id":"0baa4e0d2d18cfc454a8cf3e74733559fb67a6da"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"3d2ca7155dc7dc3e121fc724af750e20bc490e30","unresolved":true,"context_lines":[{"line_number":679,"context_line":""},{"line_number":680,"context_line":"            fixed_ip \u003d ovsport.get_ip(sub_uuid)"},{"line_number":681,"context_line":"            is_dvr_gateway_port \u003d False"},{"line_number":682,"context_line":"            is_subnet_gateway_unset \u003d subnet_info.get(\u0027gateway_ip\u0027) is None"},{"line_number":683,"context_line":"            if not is_subnet_gateway_unset and fixed_ip:"},{"line_number":684,"context_line":"                # since distributed router port must have only one fixed"},{"line_number":685,"context_line":"                # IP, directly use fixed_ip[0]"}],"source_content_type":"text/x-python","patch_set":13,"id":"c42a2a83_7817739d","line":682,"range":{"start_line":682,"start_character":12,"end_line":682,"end_character":35},"in_reply_to":"73abd4a8_f425eb9a","updated":"2020-12-15 17:15:26.000000000","message":"Agree with this","commit_id":"0baa4e0d2d18cfc454a8cf3e74733559fb67a6da"},{"author":{"_account_id":10366,"name":"Hemanth N","email":"hemanth.nakkina@canonical.com","username":"Hemanth"},"change_message_id":"d43762c0d0e287b421b8afe3f19ef6c1e047c997","unresolved":false,"context_lines":[{"line_number":679,"context_line":""},{"line_number":680,"context_line":"            fixed_ip \u003d ovsport.get_ip(sub_uuid)"},{"line_number":681,"context_line":"            is_dvr_gateway_port \u003d False"},{"line_number":682,"context_line":"            is_subnet_gateway_unset \u003d subnet_info.get(\u0027gateway_ip\u0027) is None"},{"line_number":683,"context_line":"            if not is_subnet_gateway_unset and fixed_ip:"},{"line_number":684,"context_line":"                # since distributed router port must have only one fixed"},{"line_number":685,"context_line":"                # IP, directly use fixed_ip[0]"}],"source_content_type":"text/x-python","patch_set":13,"id":"f00ca261_7d87c3aa","line":682,"range":{"start_line":682,"start_character":12,"end_line":682,"end_character":35},"in_reply_to":"c42a2a83_7817739d","updated":"2020-12-16 06:00:40.000000000","message":"Done","commit_id":"0baa4e0d2d18cfc454a8cf3e74733559fb67a6da"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"3d2ca7155dc7dc3e121fc724af750e20bc490e30","unresolved":true,"context_lines":[{"line_number":709,"context_line":"                dvr_mac\u003dself.dvr_mac_address,"},{"line_number":710,"context_line":"                rtr_port\u003dport.ofport)"},{"line_number":711,"context_line":"            if ldm.get_csnat_ofport() \u003d\u003d constants.OFPORT_INVALID and len("},{"line_number":712,"context_line":"                    ldm.get_dvr_ofports()) \u003d\u003d 1:"},{"line_number":713,"context_line":"                # if there is no csnat port for this subnet and if this is"},{"line_number":714,"context_line":"                # the last dvr port in the subnet, remove this subnet from"},{"line_number":715,"context_line":"                # local_dvr_map, as no dvr (or) csnat ports available on this"}],"source_content_type":"text/x-python","patch_set":13,"id":"8cf78ad7_e7713703","line":712,"range":{"start_line":712,"start_character":46,"end_line":712,"end_character":47},"updated":"2020-12-15 17:15:26.000000000","message":"Shouldn\u0027t this be len(...)\u003c\u003d1 ? Just to avoid any possible race condition where the dvr_ofports set is empty, for example during the agent start.","commit_id":"0baa4e0d2d18cfc454a8cf3e74733559fb67a6da"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"db95ae53b8edf5657c235a0deaeacfc7eede1f1d","unresolved":true,"context_lines":[{"line_number":708,"context_line":"                gateway_mac\u003dport.vif_mac,"},{"line_number":709,"context_line":"                dvr_mac\u003dself.dvr_mac_address,"},{"line_number":710,"context_line":"                rtr_port\u003dport.ofport)"},{"line_number":711,"context_line":"            if ldm.get_csnat_ofport() \u003d\u003d constants.OFPORT_INVALID and len("},{"line_number":712,"context_line":"                    ldm.get_dvr_ofports()) \u003d\u003d 1:"},{"line_number":713,"context_line":"                # if there is no csnat port for this subnet and if this is"},{"line_number":714,"context_line":"                # the last dvr port in the subnet, remove this subnet from"},{"line_number":715,"context_line":"                # local_dvr_map, as no dvr (or) csnat ports available on this"}],"source_content_type":"text/x-python","patch_set":13,"id":"f2134bdf_c6239b55","line":712,"range":{"start_line":711,"start_character":70,"end_line":712,"end_character":42},"updated":"2020-12-15 15:31:19.000000000","message":"nit: would be better to have len(...) on it\u0027s own line and use a parens on the if()","commit_id":"0baa4e0d2d18cfc454a8cf3e74733559fb67a6da"},{"author":{"_account_id":10366,"name":"Hemanth N","email":"hemanth.nakkina@canonical.com","username":"Hemanth"},"change_message_id":"d43762c0d0e287b421b8afe3f19ef6c1e047c997","unresolved":false,"context_lines":[{"line_number":709,"context_line":"                dvr_mac\u003dself.dvr_mac_address,"},{"line_number":710,"context_line":"                rtr_port\u003dport.ofport)"},{"line_number":711,"context_line":"            if ldm.get_csnat_ofport() \u003d\u003d constants.OFPORT_INVALID and len("},{"line_number":712,"context_line":"                    ldm.get_dvr_ofports()) \u003d\u003d 1:"},{"line_number":713,"context_line":"                # if there is no csnat port for this subnet and if this is"},{"line_number":714,"context_line":"                # the last dvr port in the subnet, remove this subnet from"},{"line_number":715,"context_line":"                # local_dvr_map, as no dvr (or) csnat ports available on this"}],"source_content_type":"text/x-python","patch_set":13,"id":"6c05941b_08e26073","line":712,"range":{"start_line":712,"start_character":46,"end_line":712,"end_character":47},"in_reply_to":"8cf78ad7_e7713703","updated":"2020-12-16 06:00:40.000000000","message":"Done","commit_id":"0baa4e0d2d18cfc454a8cf3e74733559fb67a6da"},{"author":{"_account_id":10366,"name":"Hemanth N","email":"hemanth.nakkina@canonical.com","username":"Hemanth"},"change_message_id":"d43762c0d0e287b421b8afe3f19ef6c1e047c997","unresolved":false,"context_lines":[{"line_number":708,"context_line":"                gateway_mac\u003dport.vif_mac,"},{"line_number":709,"context_line":"                dvr_mac\u003dself.dvr_mac_address,"},{"line_number":710,"context_line":"                rtr_port\u003dport.ofport)"},{"line_number":711,"context_line":"            if ldm.get_csnat_ofport() \u003d\u003d constants.OFPORT_INVALID and len("},{"line_number":712,"context_line":"                    ldm.get_dvr_ofports()) \u003d\u003d 1:"},{"line_number":713,"context_line":"                # if there is no csnat port for this subnet and if this is"},{"line_number":714,"context_line":"                # the last dvr port in the subnet, remove this subnet from"},{"line_number":715,"context_line":"                # local_dvr_map, as no dvr (or) csnat ports available on this"}],"source_content_type":"text/x-python","patch_set":13,"id":"92a0a308_5544bcbb","line":712,"range":{"start_line":711,"start_character":70,"end_line":712,"end_character":42},"in_reply_to":"f2134bdf_c6239b55","updated":"2020-12-16 06:00:40.000000000","message":"Done","commit_id":"0baa4e0d2d18cfc454a8cf3e74733559fb67a6da"}],"neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py":[{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"e39ef8f44963e55c0569ea555e5cb16ed6b7de35","unresolved":false,"context_lines":[{"line_number":3465,"context_line":"            gateway_ip \u003d \u00272001:100::1\u0027"},{"line_number":3466,"context_line":"            cidr \u003d \u00272001:100::0/64\u0027"},{"line_number":3467,"context_line":"            self._fixed_ips \u003d [{\u0027subnet_id\u0027: \u0027my-subnet-uuid\u0027,"},{"line_number":3468,"context_line":"                            \u0027ip_address\u0027: \u00272001:100::1\u0027}]"},{"line_number":3469,"context_line":"            self._fixed_ips_nongateway \u003d [{\u0027subnet_id\u0027: \u0027my-subnet-uuid\u0027,"},{"line_number":3470,"context_line":"                                       \u0027ip_address\u0027: \u00272001:100::10\u0027}]"},{"line_number":3471,"context_line":"        network_type \u003d n_const.TYPE_VXLAN"}],"source_content_type":"text/x-python","patch_set":9,"id":"1f621f24_7487afb6","line":3468,"updated":"2020-11-09 21:28:19.000000000","message":"nit: please align this line with line above","commit_id":"a7385fde3a9b6c5b8de70899c10472a68f2eeaf5"},{"author":{"_account_id":10366,"name":"Hemanth N","email":"hemanth.nakkina@canonical.com","username":"Hemanth"},"change_message_id":"c3d45630e5a6c83de86ef59233ae2e9d3333f048","unresolved":false,"context_lines":[{"line_number":3465,"context_line":"            gateway_ip \u003d \u00272001:100::1\u0027"},{"line_number":3466,"context_line":"            cidr \u003d \u00272001:100::0/64\u0027"},{"line_number":3467,"context_line":"            self._fixed_ips \u003d [{\u0027subnet_id\u0027: \u0027my-subnet-uuid\u0027,"},{"line_number":3468,"context_line":"                            \u0027ip_address\u0027: \u00272001:100::1\u0027}]"},{"line_number":3469,"context_line":"            self._fixed_ips_nongateway \u003d [{\u0027subnet_id\u0027: \u0027my-subnet-uuid\u0027,"},{"line_number":3470,"context_line":"                                       \u0027ip_address\u0027: \u00272001:100::10\u0027}]"},{"line_number":3471,"context_line":"        network_type \u003d n_const.TYPE_VXLAN"}],"source_content_type":"text/x-python","patch_set":9,"id":"85f306c0_8ed7ef89","line":3468,"in_reply_to":"1f621f24_7487afb6","updated":"2020-11-24 06:38:21.000000000","message":"Fixed in patchset 11","commit_id":"a7385fde3a9b6c5b8de70899c10472a68f2eeaf5"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"e39ef8f44963e55c0569ea555e5cb16ed6b7de35","unresolved":false,"context_lines":[{"line_number":3467,"context_line":"            self._fixed_ips \u003d [{\u0027subnet_id\u0027: \u0027my-subnet-uuid\u0027,"},{"line_number":3468,"context_line":"                            \u0027ip_address\u0027: \u00272001:100::1\u0027}]"},{"line_number":3469,"context_line":"            self._fixed_ips_nongateway \u003d [{\u0027subnet_id\u0027: \u0027my-subnet-uuid\u0027,"},{"line_number":3470,"context_line":"                                       \u0027ip_address\u0027: \u00272001:100::10\u0027}]"},{"line_number":3471,"context_line":"        network_type \u003d n_const.TYPE_VXLAN"},{"line_number":3472,"context_line":"        self._port.vif_mac \u003d gateway_mac \u003d \u0027aa:bb:cc:11:22:33\u0027"},{"line_number":3473,"context_line":"        self._port.dvr_mac \u003d self.agent.dvr_agent.dvr_mac_address"}],"source_content_type":"text/x-python","patch_set":9,"id":"1f621f24_b48dc7d4","line":3470,"updated":"2020-11-09 21:28:19.000000000","message":"same nit here","commit_id":"a7385fde3a9b6c5b8de70899c10472a68f2eeaf5"},{"author":{"_account_id":10366,"name":"Hemanth N","email":"hemanth.nakkina@canonical.com","username":"Hemanth"},"change_message_id":"c3d45630e5a6c83de86ef59233ae2e9d3333f048","unresolved":false,"context_lines":[{"line_number":3467,"context_line":"            self._fixed_ips \u003d [{\u0027subnet_id\u0027: \u0027my-subnet-uuid\u0027,"},{"line_number":3468,"context_line":"                            \u0027ip_address\u0027: \u00272001:100::1\u0027}]"},{"line_number":3469,"context_line":"            self._fixed_ips_nongateway \u003d [{\u0027subnet_id\u0027: \u0027my-subnet-uuid\u0027,"},{"line_number":3470,"context_line":"                                       \u0027ip_address\u0027: \u00272001:100::10\u0027}]"},{"line_number":3471,"context_line":"        network_type \u003d n_const.TYPE_VXLAN"},{"line_number":3472,"context_line":"        self._port.vif_mac \u003d gateway_mac \u003d \u0027aa:bb:cc:11:22:33\u0027"},{"line_number":3473,"context_line":"        self._port.dvr_mac \u003d self.agent.dvr_agent.dvr_mac_address"}],"source_content_type":"text/x-python","patch_set":9,"id":"11b4ceb1_45896942","line":3470,"in_reply_to":"1f621f24_b48dc7d4","updated":"2020-11-24 06:38:21.000000000","message":"Fixed in patchset 11","commit_id":"a7385fde3a9b6c5b8de70899c10472a68f2eeaf5"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"e39ef8f44963e55c0569ea555e5cb16ed6b7de35","unresolved":false,"context_lines":[{"line_number":3473,"context_line":"        self._port.dvr_mac \u003d self.agent.dvr_agent.dvr_mac_address"},{"line_number":3474,"context_line":"        self._compute_port.vif_mac \u003d \u002777:88:99:00:11:22\u0027"},{"line_number":3475,"context_line":"        physical_network \u003d self._physical_network"},{"line_number":3476,"context_line":"        segmentation_id \u003d self._segmentation_id"},{"line_number":3477,"context_line":"        int_br \u003d mock.create_autospec(self.agent.int_br)"},{"line_number":3478,"context_line":"        tun_br \u003d mock.create_autospec(self.agent.tun_br)"},{"line_number":3479,"context_line":"        phys_br \u003d mock.create_autospec(self.br_phys_cls(\u0027br-phys\u0027))"}],"source_content_type":"text/x-python","patch_set":9,"id":"1f621f24_f4c45ff7","line":3476,"updated":"2020-11-09 21:28:19.000000000","message":"why do You need those assignments?","commit_id":"a7385fde3a9b6c5b8de70899c10472a68f2eeaf5"},{"author":{"_account_id":10366,"name":"Hemanth N","email":"hemanth.nakkina@canonical.com","username":"Hemanth"},"change_message_id":"c3d45630e5a6c83de86ef59233ae2e9d3333f048","unresolved":true,"context_lines":[{"line_number":3473,"context_line":"        self._port.dvr_mac \u003d self.agent.dvr_agent.dvr_mac_address"},{"line_number":3474,"context_line":"        self._compute_port.vif_mac \u003d \u002777:88:99:00:11:22\u0027"},{"line_number":3475,"context_line":"        physical_network \u003d self._physical_network"},{"line_number":3476,"context_line":"        segmentation_id \u003d self._segmentation_id"},{"line_number":3477,"context_line":"        int_br \u003d mock.create_autospec(self.agent.int_br)"},{"line_number":3478,"context_line":"        tun_br \u003d mock.create_autospec(self.agent.tun_br)"},{"line_number":3479,"context_line":"        phys_br \u003d mock.create_autospec(self.br_phys_cls(\u0027br-phys\u0027))"}],"source_content_type":"text/x-python","patch_set":9,"id":"9e509586_3b75a6aa","line":3476,"in_reply_to":"1f621f24_f4c45ff7","updated":"2020-11-24 06:38:21.000000000","message":"segmentation_id is used in #L3504. Do you mean to use self._segementation_id directly instead of assigning to a different variable in this function?","commit_id":"a7385fde3a9b6c5b8de70899c10472a68f2eeaf5"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"98328ac3a05d6cfc0fe4d6556827d211feaa4e63","unresolved":true,"context_lines":[{"line_number":3473,"context_line":"        self._port.dvr_mac \u003d self.agent.dvr_agent.dvr_mac_address"},{"line_number":3474,"context_line":"        self._compute_port.vif_mac \u003d \u002777:88:99:00:11:22\u0027"},{"line_number":3475,"context_line":"        physical_network \u003d self._physical_network"},{"line_number":3476,"context_line":"        segmentation_id \u003d self._segmentation_id"},{"line_number":3477,"context_line":"        int_br \u003d mock.create_autospec(self.agent.int_br)"},{"line_number":3478,"context_line":"        tun_br \u003d mock.create_autospec(self.agent.tun_br)"},{"line_number":3479,"context_line":"        phys_br \u003d mock.create_autospec(self.br_phys_cls(\u0027br-phys\u0027))"}],"source_content_type":"text/x-python","patch_set":9,"id":"f6829a33_996afc7c","line":3476,"in_reply_to":"9e509586_3b75a6aa","updated":"2020-11-27 09:09:41.000000000","message":"It can be as it is now.","commit_id":"a7385fde3a9b6c5b8de70899c10472a68f2eeaf5"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"e39ef8f44963e55c0569ea555e5cb16ed6b7de35","unresolved":false,"context_lines":[{"line_number":3499,"context_line":"                mock.patch.object(self.agent.dvr_agent, \u0027tun_br\u0027, new\u003dtun_br),\\"},{"line_number":3500,"context_line":"                mock.patch.dict(self.agent.dvr_agent.phys_brs,"},{"line_number":3501,"context_line":"                                {physical_network: phys_br}):"},{"line_number":3502,"context_line":"            self.agent.port_bound("},{"line_number":3503,"context_line":"                self._port, self._net_uuid, network_type,"},{"line_number":3504,"context_line":"                physical_network, segmentation_id, self._fixed_ips,"},{"line_number":3505,"context_line":"                n_const.DEVICE_OWNER_DVR_INTERFACE, False)"},{"line_number":3506,"context_line":"            lvid \u003d self.agent.vlan_manager.get(self._net_uuid).vlan"},{"line_number":3507,"context_line":"            expected_on_int_br \u003d self._expected_port_bound("},{"line_number":3508,"context_line":"                self._port, lvid)"},{"line_number":3509,"context_line":"            expected_on_tun_br \u003d ["},{"line_number":3510,"context_line":"                mock.call.provision_local_vlan("},{"line_number":3511,"context_line":"                    network_type\u003dnetwork_type,"},{"line_number":3512,"context_line":"                    segmentation_id\u003dsegmentation_id,"},{"line_number":3513,"context_line":"                    lvid\u003dlvid,"},{"line_number":3514,"context_line":"                    distributed\u003dTrue),"},{"line_number":3515,"context_line":"            ] + self._expected_install_dvr_process("},{"line_number":3516,"context_line":"                port\u003dself._port,"},{"line_number":3517,"context_line":"                lvid\u003dlvid,"},{"line_number":3518,"context_line":"                ip_version\u003dip_version,"},{"line_number":3519,"context_line":"                gateway_ip\u003dgateway_ip)"},{"line_number":3520,"context_line":"            int_br.assert_has_calls(expected_on_int_br, any_order\u003dTrue)"},{"line_number":3521,"context_line":"            tun_br.assert_has_calls(expected_on_tun_br)"},{"line_number":3522,"context_line":"            phys_br.assert_not_called()"},{"line_number":3523,"context_line":"            int_br.reset_mock()"},{"line_number":3524,"context_line":"            tun_br.reset_mock()"},{"line_number":3525,"context_line":"            phys_br.reset_mock()"},{"line_number":3526,"context_line":""},{"line_number":3527,"context_line":"            self.agent.port_bound("},{"line_number":3528,"context_line":"                self._port_nongateway, self._net_uuid, network_type,"}],"source_content_type":"text/x-python","patch_set":9,"id":"1f621f24_144e3b96","line":3525,"range":{"start_line":3502,"start_character":0,"end_line":3525,"end_character":32},"updated":"2020-11-09 21:28:19.000000000","message":"for me it seems like each of those blocks like this one and similar below could be separate test - if correct name it would be much easier to read and understand what is tested there really.","commit_id":"a7385fde3a9b6c5b8de70899c10472a68f2eeaf5"},{"author":{"_account_id":10366,"name":"Hemanth N","email":"hemanth.nakkina@canonical.com","username":"Hemanth"},"change_message_id":"c3d45630e5a6c83de86ef59233ae2e9d3333f048","unresolved":false,"context_lines":[{"line_number":3499,"context_line":"                mock.patch.object(self.agent.dvr_agent, \u0027tun_br\u0027, new\u003dtun_br),\\"},{"line_number":3500,"context_line":"                mock.patch.dict(self.agent.dvr_agent.phys_brs,"},{"line_number":3501,"context_line":"                                {physical_network: phys_br}):"},{"line_number":3502,"context_line":"            self.agent.port_bound("},{"line_number":3503,"context_line":"                self._port, self._net_uuid, network_type,"},{"line_number":3504,"context_line":"                physical_network, segmentation_id, self._fixed_ips,"},{"line_number":3505,"context_line":"                n_const.DEVICE_OWNER_DVR_INTERFACE, False)"},{"line_number":3506,"context_line":"            lvid \u003d self.agent.vlan_manager.get(self._net_uuid).vlan"},{"line_number":3507,"context_line":"            expected_on_int_br \u003d self._expected_port_bound("},{"line_number":3508,"context_line":"                self._port, lvid)"},{"line_number":3509,"context_line":"            expected_on_tun_br \u003d ["},{"line_number":3510,"context_line":"                mock.call.provision_local_vlan("},{"line_number":3511,"context_line":"                    network_type\u003dnetwork_type,"},{"line_number":3512,"context_line":"                    segmentation_id\u003dsegmentation_id,"},{"line_number":3513,"context_line":"                    lvid\u003dlvid,"},{"line_number":3514,"context_line":"                    distributed\u003dTrue),"},{"line_number":3515,"context_line":"            ] + self._expected_install_dvr_process("},{"line_number":3516,"context_line":"                port\u003dself._port,"},{"line_number":3517,"context_line":"                lvid\u003dlvid,"},{"line_number":3518,"context_line":"                ip_version\u003dip_version,"},{"line_number":3519,"context_line":"                gateway_ip\u003dgateway_ip)"},{"line_number":3520,"context_line":"            int_br.assert_has_calls(expected_on_int_br, any_order\u003dTrue)"},{"line_number":3521,"context_line":"            tun_br.assert_has_calls(expected_on_tun_br)"},{"line_number":3522,"context_line":"            phys_br.assert_not_called()"},{"line_number":3523,"context_line":"            int_br.reset_mock()"},{"line_number":3524,"context_line":"            tun_br.reset_mock()"},{"line_number":3525,"context_line":"            phys_br.reset_mock()"},{"line_number":3526,"context_line":""},{"line_number":3527,"context_line":"            self.agent.port_bound("},{"line_number":3528,"context_line":"                self._port_nongateway, self._net_uuid, network_type,"}],"source_content_type":"text/x-python","patch_set":9,"id":"9049a2fa_c2c18581","line":3525,"range":{"start_line":3502,"start_character":0,"end_line":3525,"end_character":32},"in_reply_to":"1f621f24_144e3b96","updated":"2020-11-24 06:38:21.000000000","message":"This test case bounds gateway, non-gateway, compute ports and corresponding asserts are done which is not really required since they are already covered in other test cases. So removed all those checks.\nNow the assertions are only performed after the port is unbound.","commit_id":"a7385fde3a9b6c5b8de70899c10472a68f2eeaf5"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"db95ae53b8edf5657c235a0deaeacfc7eede1f1d","unresolved":true,"context_lines":[{"line_number":3449,"context_line":"            cidr \u003d \u00271.1.1.0/24\u0027"},{"line_number":3450,"context_line":"        else:"},{"line_number":3451,"context_line":"            gateway_ip \u003d \u00272001:100::1\u0027"},{"line_number":3452,"context_line":"            cidr \u003d \u00272001:100::0/64\u0027"},{"line_number":3453,"context_line":"            self._fixed_ips \u003d [{\u0027subnet_id\u0027: \u0027my-subnet-uuid\u0027,"},{"line_number":3454,"context_line":"                               \u0027ip_address\u0027: \u00272001:100::1\u0027}]"},{"line_number":3455,"context_line":"            self._fixed_ips_nongateway \u003d [{\u0027subnet_id\u0027: \u0027my-subnet-uuid\u0027,"}],"source_content_type":"text/x-python","patch_set":13,"id":"cb1b665a_0c1b459c","line":3452,"range":{"start_line":3452,"start_character":20,"end_line":3452,"end_character":29},"updated":"2020-12-15 15:31:19.000000000","message":"s/2001:db8:100::0/64 which is in the documentation prefix range, others would change accordingly","commit_id":"0baa4e0d2d18cfc454a8cf3e74733559fb67a6da"},{"author":{"_account_id":10366,"name":"Hemanth N","email":"hemanth.nakkina@canonical.com","username":"Hemanth"},"change_message_id":"d43762c0d0e287b421b8afe3f19ef6c1e047c997","unresolved":false,"context_lines":[{"line_number":3449,"context_line":"            cidr \u003d \u00271.1.1.0/24\u0027"},{"line_number":3450,"context_line":"        else:"},{"line_number":3451,"context_line":"            gateway_ip \u003d \u00272001:100::1\u0027"},{"line_number":3452,"context_line":"            cidr \u003d \u00272001:100::0/64\u0027"},{"line_number":3453,"context_line":"            self._fixed_ips \u003d [{\u0027subnet_id\u0027: \u0027my-subnet-uuid\u0027,"},{"line_number":3454,"context_line":"                               \u0027ip_address\u0027: \u00272001:100::1\u0027}]"},{"line_number":3455,"context_line":"            self._fixed_ips_nongateway \u003d [{\u0027subnet_id\u0027: \u0027my-subnet-uuid\u0027,"}],"source_content_type":"text/x-python","patch_set":13,"id":"f3544329_2e3c4a37","line":3452,"range":{"start_line":3452,"start_character":20,"end_line":3452,"end_character":29},"in_reply_to":"cb1b665a_0c1b459c","updated":"2020-12-16 06:00:40.000000000","message":"Done","commit_id":"0baa4e0d2d18cfc454a8cf3e74733559fb67a6da"}]}