)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"46d552d087320cc881529371d25b9ce9a1d29cdc","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"f8a53ba0_032be2cf","updated":"2024-02-02 08:26:37.000000000","message":"It has now a merge conflict. Plus a couple of comments. -1 for discussion","commit_id":"b7d5f49c222e5bb0beae6fb5595cb614ca8f9ea7"},{"author":{"_account_id":25468,"name":"Michel Nederlof","email":"michel@nederlof.info","username":"pellucid"},"change_message_id":"34615a3a81ec222c8fb2dca0e43ccc56ba38bbab","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"47e8b04d_1e781d2d","in_reply_to":"f8a53ba0_032be2cf","updated":"2024-02-02 10:34:13.000000000","message":"Yes, i will rebase it based on current master.","commit_id":"b7d5f49c222e5bb0beae6fb5595cb614ca8f9ea7"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"d742047bf98d0a8e3ce4d12730686ba46a87a87b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"26d5ab9e_2c4aba4f","updated":"2024-02-22 08:44:21.000000000","message":"LGTM, just what we discuss about using the new method to remove the duplicated LEAK VRF template on frr.py","commit_id":"1851444eca01700dbef226e7740878f733164e0e"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"540287d5be992fedd3d964028256a1728fd60ace","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"fab4c137_cf8e39d6","updated":"2024-02-22 11:33:13.000000000","message":"Thanks a lot for the patch! LGTM","commit_id":"5da36a2638499d1fcb222385019ae4304b18e218"},{"author":{"_account_id":25468,"name":"Michel Nederlof","email":"michel@nederlof.info","username":"pellucid"},"change_message_id":"c8784398a453c1fcc24121307c533228bcb8ad8b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"a519e907_d1b5b87b","updated":"2024-02-22 09:39:35.000000000","message":"if zuul agrees, i think this one is done :)","commit_id":"5da36a2638499d1fcb222385019ae4304b18e218"}],"ovn_bgp_agent/config.py":[{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"46d552d087320cc881529371d25b9ce9a1d29cdc","unresolved":true,"context_lines":[{"line_number":34,"context_line":"                     \u0027expose_ipv6_gua_tenant_networks flag and all tenant \u0027"},{"line_number":35,"context_line":"                     \u0027network IPs will be exposed.\u0027,"},{"line_number":36,"context_line":"                default\u003dFalse),"},{"line_number":37,"context_line":"    cfg.BoolOpt(\u0027expose_tenant_networks_per_port\u0027,"},{"line_number":38,"context_line":"                help\u003d\u0027if expose_tenant_networks is true it will (by default) \u0027"},{"line_number":39,"context_line":"                     \u0027expose all internal ports individually (so announce \u0027"},{"line_number":40,"context_line":"                     \u0027per /32 or /128). When this option is set to False, \u0027"}],"source_content_type":"text/x-python","patch_set":3,"id":"ea8e1356_9f88136c","line":37,"range":{"start_line":37,"start_character":16,"end_line":37,"end_character":50},"updated":"2024-02-02 08:26:37.000000000","message":"up until now the whole code is about exposing host ips (/32, /128), perhaps it makes more sense to add a config option named \"expose_subnet_prefix\" or \"expose_subnet_cidr\", what do you think?","commit_id":"b7d5f49c222e5bb0beae6fb5595cb614ca8f9ea7"},{"author":{"_account_id":25468,"name":"Michel Nederlof","email":"michel@nederlof.info","username":"pellucid"},"change_message_id":"c8784398a453c1fcc24121307c533228bcb8ad8b","unresolved":false,"context_lines":[{"line_number":34,"context_line":"                     \u0027expose_ipv6_gua_tenant_networks flag and all tenant \u0027"},{"line_number":35,"context_line":"                     \u0027network IPs will be exposed.\u0027,"},{"line_number":36,"context_line":"                default\u003dFalse),"},{"line_number":37,"context_line":"    cfg.BoolOpt(\u0027expose_tenant_networks_per_port\u0027,"},{"line_number":38,"context_line":"                help\u003d\u0027if expose_tenant_networks is true it will (by default) \u0027"},{"line_number":39,"context_line":"                     \u0027expose all internal ports individually (so announce \u0027"},{"line_number":40,"context_line":"                     \u0027per /32 or /128). When this option is set to False, \u0027"}],"source_content_type":"text/x-python","patch_set":3,"id":"753b48d5_ce1d003c","line":37,"range":{"start_line":37,"start_character":16,"end_line":37,"end_character":50},"in_reply_to":"ae18b1e7_2fdda83a","updated":"2024-02-22 09:39:35.000000000","message":"Done","commit_id":"b7d5f49c222e5bb0beae6fb5595cb614ca8f9ea7"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"b7ac7685a4190bf11cc5817a4a50c0d5ce99a39b","unresolved":true,"context_lines":[{"line_number":34,"context_line":"                     \u0027expose_ipv6_gua_tenant_networks flag and all tenant \u0027"},{"line_number":35,"context_line":"                     \u0027network IPs will be exposed.\u0027,"},{"line_number":36,"context_line":"                default\u003dFalse),"},{"line_number":37,"context_line":"    cfg.BoolOpt(\u0027expose_tenant_networks_per_port\u0027,"},{"line_number":38,"context_line":"                help\u003d\u0027if expose_tenant_networks is true it will (by default) \u0027"},{"line_number":39,"context_line":"                     \u0027expose all internal ports individually (so announce \u0027"},{"line_number":40,"context_line":"                     \u0027per /32 or /128). When this option is set to False, \u0027"}],"source_content_type":"text/x-python","patch_set":3,"id":"f7223a87_a5a14327","line":37,"range":{"start_line":37,"start_character":16,"end_line":37,"end_character":50},"in_reply_to":"c533d4d7_b8e5c07c","updated":"2024-02-02 10:38:24.000000000","message":"sounds good, and make default to host to not break things already working. Perhaps better to name it \"advertize_tenant_networks_method\", to better differentiate it from the underlay/ovn/vrf exposing methods","commit_id":"b7d5f49c222e5bb0beae6fb5595cb614ca8f9ea7"},{"author":{"_account_id":25468,"name":"Michel Nederlof","email":"michel@nederlof.info","username":"pellucid"},"change_message_id":"34615a3a81ec222c8fb2dca0e43ccc56ba38bbab","unresolved":true,"context_lines":[{"line_number":34,"context_line":"                     \u0027expose_ipv6_gua_tenant_networks flag and all tenant \u0027"},{"line_number":35,"context_line":"                     \u0027network IPs will be exposed.\u0027,"},{"line_number":36,"context_line":"                default\u003dFalse),"},{"line_number":37,"context_line":"    cfg.BoolOpt(\u0027expose_tenant_networks_per_port\u0027,"},{"line_number":38,"context_line":"                help\u003d\u0027if expose_tenant_networks is true it will (by default) \u0027"},{"line_number":39,"context_line":"                     \u0027expose all internal ports individually (so announce \u0027"},{"line_number":40,"context_line":"                     \u0027per /32 or /128). When this option is set to False, \u0027"}],"source_content_type":"text/x-python","patch_set":3,"id":"c533d4d7_b8e5c07c","line":37,"range":{"start_line":37,"start_character":16,"end_line":37,"end_character":50},"in_reply_to":"ea8e1356_9f88136c","updated":"2024-02-02 10:34:13.000000000","message":"Or maybe it would be better to make a selector, like \n\n`cfg.ListOpt(\u0027expose_tenant_networks_method\u0027, choices\u003d[\u0027host\u0027, \u0027subnet\u0027])`","commit_id":"b7d5f49c222e5bb0beae6fb5595cb614ca8f9ea7"},{"author":{"_account_id":25468,"name":"Michel Nederlof","email":"michel@nederlof.info","username":"pellucid"},"change_message_id":"abe1585d361b2be0402f5ccba0991c5ab15ab1c2","unresolved":true,"context_lines":[{"line_number":34,"context_line":"                     \u0027expose_ipv6_gua_tenant_networks flag and all tenant \u0027"},{"line_number":35,"context_line":"                     \u0027network IPs will be exposed.\u0027,"},{"line_number":36,"context_line":"                default\u003dFalse),"},{"line_number":37,"context_line":"    cfg.BoolOpt(\u0027expose_tenant_networks_per_port\u0027,"},{"line_number":38,"context_line":"                help\u003d\u0027if expose_tenant_networks is true it will (by default) \u0027"},{"line_number":39,"context_line":"                     \u0027expose all internal ports individually (so announce \u0027"},{"line_number":40,"context_line":"                     \u0027per /32 or /128). When this option is set to False, \u0027"}],"source_content_type":"text/x-python","patch_set":3,"id":"ae18b1e7_2fdda83a","line":37,"range":{"start_line":37,"start_character":16,"end_line":37,"end_character":50},"in_reply_to":"f7223a87_a5a14327","updated":"2024-02-02 10:47:58.000000000","message":"Yes exactly, default would be host for backward compatibility indeed. I will adapt it in the next patchset","commit_id":"b7d5f49c222e5bb0beae6fb5595cb614ca8f9ea7"}],"ovn_bgp_agent/drivers/openstack/nb_ovn_bgp_driver.py":[{"author":{"_account_id":25468,"name":"Michel Nederlof","email":"michel@nederlof.info","username":"pellucid"},"change_message_id":"787188daf1223003a9e33ecae5d0c4151db5f2f2","unresolved":true,"context_lines":[{"line_number":741,"context_line":"                        cr_lrp_info.get(\u0027bridge_device\u0027),"},{"line_number":742,"context_line":"                        cr_lrp_info.get(\u0027bridge_vlan\u0027),"},{"line_number":743,"context_line":"                        self.ovn_routing_tables, cr_lrp_info.get(\u0027ips\u0027)):"},{"line_number":744,"context_line":""},{"line_number":745,"context_line":"                    logical_switch \u003d cr_lrp_info[\u0027provider_switch\u0027]"},{"line_number":746,"context_line":"                    self._exposed_ips.setdefault(logical_switch, {}).update("},{"line_number":747,"context_line":"                        {ip: {"}],"source_content_type":"text/x-python","patch_set":1,"id":"cc720438_7a491350","line":744,"updated":"2024-01-29 13:35:10.000000000","message":"Do we need to add more calls in order to expose the cidr for exposing method underlay or ovn?","commit_id":"dd71c3f99363d75fb2fd26b93a9c55ffca3b065d"},{"author":{"_account_id":25468,"name":"Michel Nederlof","email":"michel@nederlof.info","username":"pellucid"},"change_message_id":"c8784398a453c1fcc24121307c533228bcb8ad8b","unresolved":false,"context_lines":[{"line_number":741,"context_line":"                        cr_lrp_info.get(\u0027bridge_device\u0027),"},{"line_number":742,"context_line":"                        cr_lrp_info.get(\u0027bridge_vlan\u0027),"},{"line_number":743,"context_line":"                        self.ovn_routing_tables, cr_lrp_info.get(\u0027ips\u0027)):"},{"line_number":744,"context_line":""},{"line_number":745,"context_line":"                    logical_switch \u003d cr_lrp_info[\u0027provider_switch\u0027]"},{"line_number":746,"context_line":"                    self._exposed_ips.setdefault(logical_switch, {}).update("},{"line_number":747,"context_line":"                        {ip: {"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa72eb7f_906e32ff","line":744,"in_reply_to":"7e130f3d_c286f8c7","updated":"2024-02-22 09:39:35.000000000","message":"Done","commit_id":"dd71c3f99363d75fb2fd26b93a9c55ffca3b065d"},{"author":{"_account_id":25468,"name":"Michel Nederlof","email":"michel@nederlof.info","username":"pellucid"},"change_message_id":"34615a3a81ec222c8fb2dca0e43ccc56ba38bbab","unresolved":true,"context_lines":[{"line_number":741,"context_line":"                        cr_lrp_info.get(\u0027bridge_device\u0027),"},{"line_number":742,"context_line":"                        cr_lrp_info.get(\u0027bridge_vlan\u0027),"},{"line_number":743,"context_line":"                        self.ovn_routing_tables, cr_lrp_info.get(\u0027ips\u0027)):"},{"line_number":744,"context_line":""},{"line_number":745,"context_line":"                    logical_switch \u003d cr_lrp_info[\u0027provider_switch\u0027]"},{"line_number":746,"context_line":"                    self._exposed_ips.setdefault(logical_switch, {}).update("},{"line_number":747,"context_line":"                        {ip: {"}],"source_content_type":"text/x-python","patch_set":1,"id":"7e130f3d_c286f8c7","line":744,"in_reply_to":"8addc2df_7e8e9d3f","updated":"2024-02-02 10:34:13.000000000","message":"Well it needs to be a route, adding a /24 address on the host would not work, that would still just expose a /32 in frr. \n\nI will try to add some frr config change here too, so redistribute kernel can be used too.","commit_id":"dd71c3f99363d75fb2fd26b93a9c55ffca3b065d"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"46d552d087320cc881529371d25b9ce9a1d29cdc","unresolved":true,"context_lines":[{"line_number":741,"context_line":"                        cr_lrp_info.get(\u0027bridge_device\u0027),"},{"line_number":742,"context_line":"                        cr_lrp_info.get(\u0027bridge_vlan\u0027),"},{"line_number":743,"context_line":"                        self.ovn_routing_tables, cr_lrp_info.get(\u0027ips\u0027)):"},{"line_number":744,"context_line":""},{"line_number":745,"context_line":"                    logical_switch \u003d cr_lrp_info[\u0027provider_switch\u0027]"},{"line_number":746,"context_line":"                    self._exposed_ips.setdefault(logical_switch, {}).update("},{"line_number":747,"context_line":"                        {ip: {"}],"source_content_type":"text/x-python","patch_set":1,"id":"8addc2df_7e8e9d3f","line":744,"in_reply_to":"cc720438_7a491350","updated":"2024-02-02 08:26:37.000000000","message":"we probably need to change base configuration of frr to ensure we allow exposing prefixes other tahn /32. Also, we will need to either use \"redistributed kernel\" or add the subnet IP (192.168.0.1/24) in bgp-vrf, no?","commit_id":"dd71c3f99363d75fb2fd26b93a9c55ffca3b065d"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"46d552d087320cc881529371d25b9ce9a1d29cdc","unresolved":true,"context_lines":[{"line_number":619,"context_line":"        LOG.debug(\"Added BGP route for tenant IP(s) %s on chassis %s\","},{"line_number":620,"context_line":"                  ips_to_expose, self.chassis)"},{"line_number":621,"context_line":""},{"line_number":622,"context_line":"    def _withdraw_remote_ip(self, ips, ips_info):"},{"line_number":623,"context_line":"        ips_to_withdraw \u003d ips"},{"line_number":624,"context_line":"        if not CONF.expose_tenant_networks:"},{"line_number":625,"context_line":"            # This means CONF.expose_ipv6_gua_tenant_networks is enabled"}],"source_content_type":"text/x-python","patch_set":3,"id":"f5285279_d74858e4","line":622,"range":{"start_line":622,"start_character":0,"end_line":622,"end_character":49},"updated":"2024-02-02 08:26:37.000000000","message":"do you need to also check this to skip it?","commit_id":"b7d5f49c222e5bb0beae6fb5595cb614ca8f9ea7"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"b7ac7685a4190bf11cc5817a4a50c0d5ce99a39b","unresolved":true,"context_lines":[{"line_number":619,"context_line":"        LOG.debug(\"Added BGP route for tenant IP(s) %s on chassis %s\","},{"line_number":620,"context_line":"                  ips_to_expose, self.chassis)"},{"line_number":621,"context_line":""},{"line_number":622,"context_line":"    def _withdraw_remote_ip(self, ips, ips_info):"},{"line_number":623,"context_line":"        ips_to_withdraw \u003d ips"},{"line_number":624,"context_line":"        if not CONF.expose_tenant_networks:"},{"line_number":625,"context_line":"            # This means CONF.expose_ipv6_gua_tenant_networks is enabled"}],"source_content_type":"text/x-python","patch_set":3,"id":"b2accbd6_2323defe","line":622,"range":{"start_line":622,"start_character":0,"end_line":622,"end_character":49},"in_reply_to":"a3ebcbb9_63d16622","updated":"2024-02-02 10:38:24.000000000","message":"that is my point... it is missing the \"if not CONF...\", I think you should add it. Or am I missing somthing?","commit_id":"b7d5f49c222e5bb0beae6fb5595cb614ca8f9ea7"},{"author":{"_account_id":25468,"name":"Michel Nederlof","email":"michel@nederlof.info","username":"pellucid"},"change_message_id":"abe1585d361b2be0402f5ccba0991c5ab15ab1c2","unresolved":true,"context_lines":[{"line_number":619,"context_line":"        LOG.debug(\"Added BGP route for tenant IP(s) %s on chassis %s\","},{"line_number":620,"context_line":"                  ips_to_expose, self.chassis)"},{"line_number":621,"context_line":""},{"line_number":622,"context_line":"    def _withdraw_remote_ip(self, ips, ips_info):"},{"line_number":623,"context_line":"        ips_to_withdraw \u003d ips"},{"line_number":624,"context_line":"        if not CONF.expose_tenant_networks:"},{"line_number":625,"context_line":"            # This means CONF.expose_ipv6_gua_tenant_networks is enabled"}],"source_content_type":"text/x-python","patch_set":3,"id":"d27b6c10_d619c8e4","line":622,"range":{"start_line":622,"start_character":0,"end_line":622,"end_character":49},"in_reply_to":"b2accbd6_2323defe","updated":"2024-02-02 10:47:58.000000000","message":"You are absolutely right, i will add it here too.","commit_id":"b7d5f49c222e5bb0beae6fb5595cb614ca8f9ea7"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"d742047bf98d0a8e3ce4d12730686ba46a87a87b","unresolved":false,"context_lines":[{"line_number":619,"context_line":"        LOG.debug(\"Added BGP route for tenant IP(s) %s on chassis %s\","},{"line_number":620,"context_line":"                  ips_to_expose, self.chassis)"},{"line_number":621,"context_line":""},{"line_number":622,"context_line":"    def _withdraw_remote_ip(self, ips, ips_info):"},{"line_number":623,"context_line":"        ips_to_withdraw \u003d ips"},{"line_number":624,"context_line":"        if not CONF.expose_tenant_networks:"},{"line_number":625,"context_line":"            # This means CONF.expose_ipv6_gua_tenant_networks is enabled"}],"source_content_type":"text/x-python","patch_set":3,"id":"fd5024ee_bebcb5b1","line":622,"range":{"start_line":622,"start_character":0,"end_line":622,"end_character":49},"in_reply_to":"d27b6c10_d619c8e4","updated":"2024-02-22 08:44:21.000000000","message":"Done","commit_id":"b7d5f49c222e5bb0beae6fb5595cb614ca8f9ea7"},{"author":{"_account_id":25468,"name":"Michel Nederlof","email":"michel@nederlof.info","username":"pellucid"},"change_message_id":"34615a3a81ec222c8fb2dca0e43ccc56ba38bbab","unresolved":true,"context_lines":[{"line_number":619,"context_line":"        LOG.debug(\"Added BGP route for tenant IP(s) %s on chassis %s\","},{"line_number":620,"context_line":"                  ips_to_expose, self.chassis)"},{"line_number":621,"context_line":""},{"line_number":622,"context_line":"    def _withdraw_remote_ip(self, ips, ips_info):"},{"line_number":623,"context_line":"        ips_to_withdraw \u003d ips"},{"line_number":624,"context_line":"        if not CONF.expose_tenant_networks:"},{"line_number":625,"context_line":"            # This means CONF.expose_ipv6_gua_tenant_networks is enabled"}],"source_content_type":"text/x-python","patch_set":3,"id":"a3ebcbb9_63d16622","line":622,"range":{"start_line":622,"start_character":0,"end_line":622,"end_character":49},"in_reply_to":"f5285279_d74858e4","updated":"2024-02-02 10:34:13.000000000","message":"events might still match, so that\u0027s why i added it here too.","commit_id":"b7d5f49c222e5bb0beae6fb5595cb614ca8f9ea7"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"540287d5be992fedd3d964028256a1728fd60ace","unresolved":true,"context_lines":[{"line_number":153,"context_line":"                           watcher.ChassisRedirectDeleteEvent(self),"},{"line_number":154,"context_line":"                           watcher.LogicalSwitchPortSubnetAttachEvent(self),"},{"line_number":155,"context_line":"                           watcher.LogicalSwitchPortSubnetDetachEvent(self)})"},{"line_number":156,"context_line":"            if CONF.advertisement_method_tenant_networks \u003d\u003d \u0027host\u0027:"},{"line_number":157,"context_line":"                events.update({"},{"line_number":158,"context_line":"                    watcher.LogicalSwitchPortTenantCreateEvent(self),"},{"line_number":159,"context_line":"                    watcher.LogicalSwitchPortTenantDeleteEvent(self)"}],"source_content_type":"text/x-python","patch_set":8,"id":"68a3ae40_c12bdb33","line":156,"range":{"start_line":156,"start_character":60,"end_line":156,"end_character":66},"updated":"2024-02-22 11:33:13.000000000","message":"Nit: Perhaps the \"host\" and \"subnet\" values should be constants to avoid potential typos","commit_id":"5da36a2638499d1fcb222385019ae4304b18e218"}]}