)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":25468,"name":"Michel Nederlof","email":"michel@nederlof.info","username":"pellucid"},"change_message_id":"bfb22cc46b288633faeb7aede798203f2cf72597","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"33e0d7c7_8c9172d3","updated":"2025-04-14 13:31:25.000000000","message":"I think you are almost there, but it needs more thought into the live migration process and how that is handled.","commit_id":"3ec98ab2fcc9cc52b149c1197492777f873bfdce"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"bcbfa84bb265d8790ec1129c140a9fe85e3cabc4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"5db9fc1c_4b34c798","updated":"2025-04-14 14:00:13.000000000","message":"Thanks Michel for the great review!","commit_id":"3ec98ab2fcc9cc52b149c1197492777f873bfdce"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"b170add71941f36043c6903889541106cb5aa2ea","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"883353ca_f805a8c9","updated":"2025-03-31 14:10:06.000000000","message":"bump - ping for reviews","commit_id":"3ec98ab2fcc9cc52b149c1197492777f873bfdce"},{"author":{"_account_id":24824,"name":"Dmitrii Shcherbakov","username":"dmitriis"},"change_message_id":"d0c322dfb6a609bd9737a38626dccb02b5b56c63","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"e0a6ce3b_7719cde8","updated":"2025-05-07 22:25:17.000000000","message":"Did some functional testing on my end - create/delete operations seem to work correctly.\n\nAlso in favor of this change since with the old LSP-based code I keep encountering a relatively rare condition where the cluster-wide ovn-nb does not contain `neutron:port_fip` external-id for some LSPs that have FIPs associated in Neutron. Looking at the ovn-nb, there are relevant diff transactions that are supposed to be setting it but still it\u0027s gone without the corresponding removal transactions. ovn-bgp-agent reacts to the lack of `neutron:port_fip` and unwires the IP when it\u0027s supposed to be wired. At the same time the relevant NAT table entries exist. In the local ovn-nb the static_mac_binding is created and is then gone after \u003c200 ms because ovn-bgp-agent creates and then deletes this state making the VM unreachable.\n\nWatching the NAT table seems to avoid this altogether.","commit_id":"719ac919be37d81d9b935b0e60f6335e27289831"}],"ovn_bgp_agent/drivers/openstack/watchers/base_watcher.py":[{"author":{"_account_id":25468,"name":"Michel Nederlof","email":"michel@nederlof.info","username":"pellucid"},"change_message_id":"bfb22cc46b288633faeb7aede798203f2cf72597","unresolved":true,"context_lines":[{"line_number":129,"context_line":"                          \u0027nat\u0027: row.uuid})"},{"line_number":130,"context_line":"            return False"},{"line_number":131,"context_line":""},{"line_number":132,"context_line":"        if lsp.type \u003d\u003d constants.OVN_VIRTUAL_VIF_PORT_TYPE:"},{"line_number":133,"context_line":"            # The host it should be exposed to is set in the external_ids"},{"line_number":134,"context_line":"            lsp_host \u003d lsp.external_ids.get(constants.OVN_HOST_ID_EXT_ID_KEY)"},{"line_number":135,"context_line":"            if lsp_host !\u003d self.agent.chassis:"}],"source_content_type":"text/x-python","patch_set":5,"id":"ff1aed06_e4930e4b","line":132,"updated":"2025-04-14 13:31:25.000000000","message":"Why not use \n\n```\nlsp_current_host \u003d driver_utils.get_port_chassis(lsp, self.agent.chassis)\nreturn lsp_current_host \u003d\u003d self.agent.chassis\n```\n\ninstead of the if/else structure you created here?","commit_id":"3ec98ab2fcc9cc52b149c1197492777f873bfdce"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"7e4a0e49b40915c65ae449799a13eb97dcceb87c","unresolved":false,"context_lines":[{"line_number":129,"context_line":"                          \u0027nat\u0027: row.uuid})"},{"line_number":130,"context_line":"            return False"},{"line_number":131,"context_line":""},{"line_number":132,"context_line":"        if lsp.type \u003d\u003d constants.OVN_VIRTUAL_VIF_PORT_TYPE:"},{"line_number":133,"context_line":"            # The host it should be exposed to is set in the external_ids"},{"line_number":134,"context_line":"            lsp_host \u003d lsp.external_ids.get(constants.OVN_HOST_ID_EXT_ID_KEY)"},{"line_number":135,"context_line":"            if lsp_host !\u003d self.agent.chassis:"}],"source_content_type":"text/x-python","patch_set":5,"id":"62bf0141_de960d2a","line":132,"in_reply_to":"a4664494_d8a219c3","updated":"2025-04-15 20:17:20.000000000","message":"Done","commit_id":"3ec98ab2fcc9cc52b149c1197492777f873bfdce"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"bcbfa84bb265d8790ec1129c140a9fe85e3cabc4","unresolved":true,"context_lines":[{"line_number":129,"context_line":"                          \u0027nat\u0027: row.uuid})"},{"line_number":130,"context_line":"            return False"},{"line_number":131,"context_line":""},{"line_number":132,"context_line":"        if lsp.type \u003d\u003d constants.OVN_VIRTUAL_VIF_PORT_TYPE:"},{"line_number":133,"context_line":"            # The host it should be exposed to is set in the external_ids"},{"line_number":134,"context_line":"            lsp_host \u003d lsp.external_ids.get(constants.OVN_HOST_ID_EXT_ID_KEY)"},{"line_number":135,"context_line":"            if lsp_host !\u003d self.agent.chassis:"}],"source_content_type":"text/x-python","patch_set":5,"id":"a4664494_d8a219c3","line":132,"in_reply_to":"ff1aed06_e4930e4b","updated":"2025-04-14 14:00:13.000000000","message":"Yep, thanks, that\u0027s better than what I wrote.","commit_id":"3ec98ab2fcc9cc52b149c1197492777f873bfdce"}],"ovn_bgp_agent/drivers/openstack/watchers/nb_bgp_watcher.py":[{"author":{"_account_id":25468,"name":"Michel Nederlof","email":"michel@nederlof.info","username":"pellucid"},"change_message_id":"bfb22cc46b288633faeb7aede798203f2cf72597","unresolved":true,"context_lines":[{"line_number":257,"context_line":"            self.agent.expose_fip(external_ip, external_mac, ls_name, row)"},{"line_number":258,"context_line":""},{"line_number":259,"context_line":""},{"line_number":260,"context_line":"class LogicalSwitchPortFIPDeleteEvent(base_watcher.LSPChassisEvent):"},{"line_number":261,"context_line":"    \u0027\u0027\u0027Floating IP delete events based on the LogicalSwitchPort"},{"line_number":262,"context_line":""},{"line_number":263,"context_line":"    The LSP has information about the host is should be exposed to, which"}],"source_content_type":"text/x-python","patch_set":5,"id":"a173b250_af9758c5","side":"PARENT","line":260,"range":{"start_line":260,"start_character":6,"end_line":260,"end_character":37},"updated":"2025-04-14 13:31:25.000000000","message":"This class (and also the `LogicalSwitchPortFIPCreateEvent` class) also handled floating ip mobility (e.g. lsp moved from one host to another), how is this handled with the other two classes (as the Nat table is not updated during live migrate)","commit_id":"65b5913fddc87b9e41b3519c8d46b597677c810f"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"bcbfa84bb265d8790ec1129c140a9fe85e3cabc4","unresolved":true,"context_lines":[{"line_number":257,"context_line":"            self.agent.expose_fip(external_ip, external_mac, ls_name, row)"},{"line_number":258,"context_line":""},{"line_number":259,"context_line":""},{"line_number":260,"context_line":"class LogicalSwitchPortFIPDeleteEvent(base_watcher.LSPChassisEvent):"},{"line_number":261,"context_line":"    \u0027\u0027\u0027Floating IP delete events based on the LogicalSwitchPort"},{"line_number":262,"context_line":""},{"line_number":263,"context_line":"    The LSP has information about the host is should be exposed to, which"}],"source_content_type":"text/x-python","patch_set":5,"id":"0eae7488_5f17db10","side":"PARENT","line":260,"range":{"start_line":260,"start_character":6,"end_line":260,"end_character":37},"in_reply_to":"a173b250_af9758c5","updated":"2025-04-14 14:00:13.000000000","message":"Great catch! I will need to look into it. Given that our CI is running a single compute it was not able to catch this goof of mine.","commit_id":"65b5913fddc87b9e41b3519c8d46b597677c810f"},{"author":{"_account_id":25468,"name":"Michel Nederlof","email":"michel@nederlof.info","username":"pellucid"},"change_message_id":"bfb22cc46b288633faeb7aede798203f2cf72597","unresolved":true,"context_lines":[{"line_number":740,"context_line":"              base_watcher.DnatSnatBaseEvent.ROW_DELETE)"},{"line_number":741,"context_line":""},{"line_number":742,"context_line":"    def match_fn(self, event, row, old):"},{"line_number":743,"context_line":"        if not super().match_fn(event, row, old):"},{"line_number":744,"context_line":"            return False"},{"line_number":745,"context_line":""},{"line_number":746,"context_line":"        if not row.external_ip:"}],"source_content_type":"text/x-python","patch_set":5,"id":"6e505a47_b5590585","line":743,"updated":"2025-04-14 13:31:25.000000000","message":"do we know for sure, that if a port is deleted with a floating ip attached, then the lsp is still available? Because without the LSP information you cannot determine if it was attached to this chassis.\n\nMaybe also check if the `row.external_ip` returns true from `self.agent.is_ip_exposed` ?\n```\n        if event \u003d\u003d self.ROW_DELETE:\n            logical_switch \u003d common_utils.get_from_external_ids(\n                row, constants.OVN_LS_NAME_EXT_ID_KEY)\n            is_exposed \u003d self.agent.is_ip_exposed(logical_switch,\n                                                  row.external_ip)\n```","commit_id":"3ec98ab2fcc9cc52b149c1197492777f873bfdce"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"7e4a0e49b40915c65ae449799a13eb97dcceb87c","unresolved":true,"context_lines":[{"line_number":740,"context_line":"              base_watcher.DnatSnatBaseEvent.ROW_DELETE)"},{"line_number":741,"context_line":""},{"line_number":742,"context_line":"    def match_fn(self, event, row, old):"},{"line_number":743,"context_line":"        if not super().match_fn(event, row, old):"},{"line_number":744,"context_line":"            return False"},{"line_number":745,"context_line":""},{"line_number":746,"context_line":"        if not row.external_ip:"}],"source_content_type":"text/x-python","patch_set":5,"id":"64b6a3f5_f4083fc1","line":743,"in_reply_to":"1b717917_b26d8ad5","updated":"2025-04-15 20:17:20.000000000","message":"I forgot, since it\u0027s been a while since I wrote this patch :D The NAT entry in Neutron exists only if the FIP is associated with a port - so Neutron deletes the NAT is the LSP is deleted. So we\u0027re good here.","commit_id":"3ec98ab2fcc9cc52b149c1197492777f873bfdce"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"bcbfa84bb265d8790ec1129c140a9fe85e3cabc4","unresolved":true,"context_lines":[{"line_number":740,"context_line":"              base_watcher.DnatSnatBaseEvent.ROW_DELETE)"},{"line_number":741,"context_line":""},{"line_number":742,"context_line":"    def match_fn(self, event, row, old):"},{"line_number":743,"context_line":"        if not super().match_fn(event, row, old):"},{"line_number":744,"context_line":"            return False"},{"line_number":745,"context_line":""},{"line_number":746,"context_line":"        if not row.external_ip:"}],"source_content_type":"text/x-python","patch_set":5,"id":"feb87c1e_f3b96c4f","line":743,"in_reply_to":"6e505a47_b5590585","updated":"2025-04-14 14:00:13.000000000","message":"The LSP is gone if we delete a port. I think I overlooked this - the code on L750 an below is handling that but it never gets there.\n\nAs for the `is_ip_exposed()` is concerned - I\u0027d like to get rid of those caches because they do not make any sense from the arch POV. The OVN DB should be the source of truth and the cache doesn\u0027t speed up anything.","commit_id":"3ec98ab2fcc9cc52b149c1197492777f873bfdce"},{"author":{"_account_id":25468,"name":"Michel Nederlof","email":"michel@nederlof.info","username":"pellucid"},"change_message_id":"8e54090781903bcdf72fbd217a3d811cef7eea7d","unresolved":true,"context_lines":[{"line_number":740,"context_line":"              base_watcher.DnatSnatBaseEvent.ROW_DELETE)"},{"line_number":741,"context_line":""},{"line_number":742,"context_line":"    def match_fn(self, event, row, old):"},{"line_number":743,"context_line":"        if not super().match_fn(event, row, old):"},{"line_number":744,"context_line":"            return False"},{"line_number":745,"context_line":""},{"line_number":746,"context_line":"        if not row.external_ip:"}],"source_content_type":"text/x-python","patch_set":5,"id":"1b717917_b26d8ad5","line":743,"in_reply_to":"feb87c1e_f3b96c4f","updated":"2025-04-15 08:30:22.000000000","message":"For the `is_ip_exposed` comment; i do not disagree on the fact that you rather not use it, but (further testing is required obviously), but if a delete comes around for a specific ip (and the other required records have been deleted already), then we do need a way to get rid of the announcement (or find the right place/event obviously O:-) )","commit_id":"3ec98ab2fcc9cc52b149c1197492777f873bfdce"}]}