)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"6c0f34380944b715b1033aafb19089e21f135f5e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"1a974e2c_51457789","updated":"2023-08-16 10:16:17.000000000","message":"A reno is needed#\n\nI\u0027ve also created https://bugs.launchpad.net/neutron/+bug/2031520 to document all the router port \"stuff\" related to dvr/centralized depending on the network type, the config options we need to add, the presence or not of PF, etc. For someone not aware of this, could be very complicated.","commit_id":"0843e5be8a1589b0c3919985e0bf36a35bd690f4"},{"author":{"_account_id":8313,"name":"Lajos Katona","display_name":"lajoskatona","email":"katonalala@gmail.com","username":"elajkat","status":"Ericsson Software Technology"},"change_message_id":"9b0c10fba7d3275ca98ca47be9d8427db4054d57","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"b775def9_10f572a5","updated":"2023-08-04 12:55:26.000000000","message":"As I see it looks ok, and the comments with some more reading made my understanding better of this topic :-)","commit_id":"0843e5be8a1589b0c3919985e0bf36a35bd690f4"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"db4e021450bc3436260d2c531de6cb66dc10af9f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"015f2df8_e36bb6de","updated":"2023-08-21 10:35:20.000000000","message":"Adding -1 just for discussion\n\nI have just a nit about this checking the LBs not just the PFs. Also, I suppose this will also require adaptations in the ovn-octavia to ensure the same is done there, right? \n\nAlso, not sure what would be the implications in the ongoing traffic for other VMs when transitioning to/from distributed to centralized and viceversa","commit_id":"a2433a4514237d3bdf562e43042cd396771e6326"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"e4c4b29ab74d741915029df4b186cce47bce65d4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"62f15235_f4a17182","updated":"2023-08-23 10:27:04.000000000","message":"It indeed has some issues with Floating IPs so I need to spent more time on it definitely","commit_id":"a2433a4514237d3bdf562e43042cd396771e6326"}],"neutron/common/ovn/utils.py":[{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"6c0f34380944b715b1033aafb19089e21f135f5e","unresolved":true,"context_lines":[{"line_number":638,"context_line":""},{"line_number":639,"context_line":""},{"line_number":640,"context_line":"def is_provider_network(network):"},{"line_number":641,"context_line":"    # NOTE(slaweq): In some cases it may be that just network_id is given"},{"line_number":642,"context_line":"    if isinstance(network, str):"},{"line_number":643,"context_line":"        ctx \u003d n_context.get_admin_context()"},{"line_number":644,"context_line":"        plugin \u003d directory.get_plugin()"}],"source_content_type":"text/x-python","patch_set":2,"id":"11948592_d597dd21","line":641,"range":{"start_line":641,"start_character":0,"end_line":641,"end_character":27},"updated":"2023-08-16 10:16:17.000000000","message":"nit: better add method description with :param definition. E.g.:\n  :param network: (str, dict)","commit_id":"0843e5be8a1589b0c3919985e0bf36a35bd690f4"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"7c410ccda2d4bde8f65d1140f28ab895f9424d65","unresolved":false,"context_lines":[{"line_number":638,"context_line":""},{"line_number":639,"context_line":""},{"line_number":640,"context_line":"def is_provider_network(network):"},{"line_number":641,"context_line":"    # NOTE(slaweq): In some cases it may be that just network_id is given"},{"line_number":642,"context_line":"    if isinstance(network, str):"},{"line_number":643,"context_line":"        ctx \u003d n_context.get_admin_context()"},{"line_number":644,"context_line":"        plugin \u003d directory.get_plugin()"}],"source_content_type":"text/x-python","patch_set":2,"id":"691f0b74_461ab586","line":641,"range":{"start_line":641,"start_character":0,"end_line":641,"end_character":27},"in_reply_to":"11948592_d597dd21","updated":"2023-08-16 12:45:50.000000000","message":"Done","commit_id":"0843e5be8a1589b0c3919985e0bf36a35bd690f4"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"0f717709a7b3b23b4b15b32f048a580037c2ffbe","unresolved":true,"context_lines":[{"line_number":651,"context_line":"        try:"},{"line_number":652,"context_line":"            network \u003d plugin.get_network(ctx, network)"},{"line_number":653,"context_line":"        except n_exc.NetworkNotFound:"},{"line_number":654,"context_line":"            LOG.warning(\"Network %s not found in the database. \", network)"},{"line_number":655,"context_line":"            return False"},{"line_number":656,"context_line":"    return network.get(provider_net.PHYSICAL_NETWORK, False)"},{"line_number":657,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"41afa8e8_d810607d","line":654,"updated":"2023-08-17 14:23:54.000000000","message":"I realize this shouldn\u0027t happen, but does the log get us anything?","commit_id":"a2433a4514237d3bdf562e43042cd396771e6326"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"573e8431c5d7689f699aadd69e8d6ab82143b3bc","unresolved":true,"context_lines":[{"line_number":651,"context_line":"        try:"},{"line_number":652,"context_line":"            network \u003d plugin.get_network(ctx, network)"},{"line_number":653,"context_line":"        except n_exc.NetworkNotFound:"},{"line_number":654,"context_line":"            LOG.warning(\"Network %s not found in the database. \", network)"},{"line_number":655,"context_line":"            return False"},{"line_number":656,"context_line":"    return network.get(provider_net.PHYSICAL_NETWORK, False)"},{"line_number":657,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"1b46dc60_04c5f76a","line":654,"in_reply_to":"41afa8e8_d810607d","updated":"2023-08-21 09:19:47.000000000","message":"TBH I\u0027m not sure but I wanted to be on the \"safe side\" and have it logged in case it would happen.","commit_id":"a2433a4514237d3bdf562e43042cd396771e6326"}],"neutron/services/portforwarding/drivers/ovn/driver.py":[{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"6c0f34380944b715b1033aafb19089e21f135f5e","unresolved":true,"context_lines":[{"line_number":145,"context_line":"                ext_id_key \u003d ovn_const.OVN_NETWORK_NAME_EXT_ID_KEY"},{"line_number":146,"context_line":"                if (port.external_ids.get(ext_id_key) and"},{"line_number":147,"context_line":"                        not port.gateway_chassis):"},{"line_number":148,"context_line":"                    ls_name \u003d port.external_ids.get(ext_id_key)"},{"line_number":149,"context_line":"                    try:"},{"line_number":150,"context_line":"                        ovn_txn.add(nb_ovn.ls_lb_add(ls_name, lb_name,"},{"line_number":151,"context_line":"                                                     may_exist\u003dTrue))"}],"source_content_type":"text/x-python","patch_set":2,"id":"bc39b7be_68548562","line":148,"range":{"start_line":148,"start_character":20,"end_line":148,"end_character":63},"updated":"2023-08-16 10:16:17.000000000","message":"Most probably we\u0027ll have multiple ports in the same network, so L150 could be executed needed-less several times. You can maybe store the network names in a list.","commit_id":"0843e5be8a1589b0c3919985e0bf36a35bd690f4"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"577e618e41293d48c22fe43414063e24df701932","unresolved":true,"context_lines":[{"line_number":145,"context_line":"                ext_id_key \u003d ovn_const.OVN_NETWORK_NAME_EXT_ID_KEY"},{"line_number":146,"context_line":"                if (port.external_ids.get(ext_id_key) and"},{"line_number":147,"context_line":"                        not port.gateway_chassis):"},{"line_number":148,"context_line":"                    ls_name \u003d port.external_ids.get(ext_id_key)"},{"line_number":149,"context_line":"                    try:"},{"line_number":150,"context_line":"                        ovn_txn.add(nb_ovn.ls_lb_add(ls_name, lb_name,"},{"line_number":151,"context_line":"                                                     may_exist\u003dTrue))"}],"source_content_type":"text/x-python","patch_set":2,"id":"cc28028e_fdc6e9cb","line":148,"range":{"start_line":148,"start_character":20,"end_line":148,"end_character":63},"in_reply_to":"80c6f2ab_d241160d","updated":"2023-08-17 13:48:26.000000000","message":"No, what I was saying is that if you have several router ports, probably lot of them will belong to the same network. That means you\u0027ll repeat the \"nb_ovn.ls_lb_add(ls_name, lb_name, may_exist\u003dTrue)\" command many times unnecessarily. ovsdbapp will not issue a DB request if the value is the same, but this could be considered as an optimization for future patches.","commit_id":"0843e5be8a1589b0c3919985e0bf36a35bd690f4"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"7c410ccda2d4bde8f65d1140f28ab895f9424d65","unresolved":true,"context_lines":[{"line_number":145,"context_line":"                ext_id_key \u003d ovn_const.OVN_NETWORK_NAME_EXT_ID_KEY"},{"line_number":146,"context_line":"                if (port.external_ids.get(ext_id_key) and"},{"line_number":147,"context_line":"                        not port.gateway_chassis):"},{"line_number":148,"context_line":"                    ls_name \u003d port.external_ids.get(ext_id_key)"},{"line_number":149,"context_line":"                    try:"},{"line_number":150,"context_line":"                        ovn_txn.add(nb_ovn.ls_lb_add(ls_name, lb_name,"},{"line_number":151,"context_line":"                                                     may_exist\u003dTrue))"}],"source_content_type":"text/x-python","patch_set":2,"id":"80c6f2ab_d241160d","line":148,"range":{"start_line":148,"start_character":20,"end_line":148,"end_character":63},"in_reply_to":"bc39b7be_68548562","updated":"2023-08-16 12:45:50.000000000","message":"but all that info is already stored in the \"port\" object so do I really need to have additional dict?","commit_id":"0843e5be8a1589b0c3919985e0bf36a35bd690f4"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"db4e021450bc3436260d2c531de6cb66dc10af9f","unresolved":true,"context_lines":[{"line_number":185,"context_line":"            self._port_forwarding_created(ovn_txn, nb_ovn, pf_obj,"},{"line_number":186,"context_line":"                                          is_range\u003dis_range)"},{"line_number":187,"context_line":""},{"line_number":188,"context_line":"    def _set_reside_redir_for_lr_without_pf(self, ovn_txn, nb_ovn, ovn_lr):"},{"line_number":189,"context_line":"        if len(ovn_lr.load_balancer) \u003e 1:"},{"line_number":190,"context_line":"            # NOTE(slaweq): if there are still other LBs associated to the LR"},{"line_number":191,"context_line":"            # then there is nothing to change there"}],"source_content_type":"text/x-python","patch_set":3,"id":"7254bdd7_37f34752","line":188,"range":{"start_line":188,"start_character":8,"end_line":188,"end_character":43},"updated":"2023-08-21 10:35:20.000000000","message":"this is actually not checking if there are PF but if there are loadbalancers, perhaps worth a note to clarify and to ensure this works fine together with ovn-octavia","commit_id":"a2433a4514237d3bdf562e43042cd396771e6326"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"e4c4b29ab74d741915029df4b186cce47bce65d4","unresolved":true,"context_lines":[{"line_number":185,"context_line":"            self._port_forwarding_created(ovn_txn, nb_ovn, pf_obj,"},{"line_number":186,"context_line":"                                          is_range\u003dis_range)"},{"line_number":187,"context_line":""},{"line_number":188,"context_line":"    def _set_reside_redir_for_lr_without_pf(self, ovn_txn, nb_ovn, ovn_lr):"},{"line_number":189,"context_line":"        if len(ovn_lr.load_balancer) \u003e 1:"},{"line_number":190,"context_line":"            # NOTE(slaweq): if there are still other LBs associated to the LR"},{"line_number":191,"context_line":"            # then there is nothing to change there"}],"source_content_type":"text/x-python","patch_set":3,"id":"b938a331_93b22b29","line":188,"range":{"start_line":188,"start_character":8,"end_line":188,"end_character":43},"in_reply_to":"7254bdd7_37f34752","updated":"2023-08-23 10:27:04.000000000","message":"you\u0027re right but it\u0027s like that everywhere in this file so I think it should be clear why we are looking for LBs in this case.\nRegarding ovn-octavia loadbalancers which may also be associater with this LR, in such case reside_redir_chassis option will remain to be set to \"true\" after last PF will be removed as there will be still other LB associated to the LR. I don\u0027t think we need to do anything more here really.","commit_id":"a2433a4514237d3bdf562e43042cd396771e6326"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"fe8cdc51c7b93dd1bab32cef9e7f42f6afe68762","unresolved":true,"context_lines":[{"line_number":185,"context_line":"            self._port_forwarding_created(ovn_txn, nb_ovn, pf_obj,"},{"line_number":186,"context_line":"                                          is_range\u003dis_range)"},{"line_number":187,"context_line":""},{"line_number":188,"context_line":"    def _set_reside_redir_for_lr_without_pf(self, ovn_txn, nb_ovn, ovn_lr):"},{"line_number":189,"context_line":"        if len(ovn_lr.load_balancer) \u003e 1:"},{"line_number":190,"context_line":"            # NOTE(slaweq): if there are still other LBs associated to the LR"},{"line_number":191,"context_line":"            # then there is nothing to change there"}],"source_content_type":"text/x-python","patch_set":3,"id":"118c6481_e88b453e","line":188,"range":{"start_line":188,"start_character":8,"end_line":188,"end_character":43},"in_reply_to":"b938a331_93b22b29","updated":"2023-08-23 11:29:23.000000000","message":"I agree, probably what we need to do is to also include this type of checking in ovn-octavia.","commit_id":"a2433a4514237d3bdf562e43042cd396771e6326"}]}