)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"09e56ffefb80f0ca2c5bec508b13e66333556259","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"b75e8468_19ddb668","updated":"2023-07-31 01:23:12.000000000","message":"The code is basically fine to me, except for an unnecessary refactoring, IMO.","commit_id":"432315978607730fea329269722af1499473ebea"}],"neutron/plugins/ml2/drivers/openvswitch/agent/openflow/native/br_tun.py":[{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"09e56ffefb80f0ca2c5bec508b13e66333556259","unresolved":true,"context_lines":[{"line_number":37,"context_line":"            self, patch_int_ofport, arp_responder_enabled, dvr_enabled):"},{"line_number":38,"context_line":"        (dp, ofp, ofpp) \u003d self._get_dp()"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"        if not dvr_enabled:"},{"line_number":41,"context_line":"            # Table 0 (default) will sort incoming traffic depending on in_port"},{"line_number":42,"context_line":"            # This table is needed only for non-dvr environment because"},{"line_number":43,"context_line":"            # OVSDVRProcessMixin overwrites this flow in its"},{"line_number":44,"context_line":"            # install_dvr_process() method."},{"line_number":45,"context_line":"            self.install_goto(dest_table_id\u003dconstants.PATCH_LV_TO_TUN,"},{"line_number":46,"context_line":"                              priority\u003d1,"},{"line_number":47,"context_line":"                              in_port\u003dpatch_int_ofport)"},{"line_number":48,"context_line":""},{"line_number":49,"context_line":"        self.install_drop()  # default drop"},{"line_number":50,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"87874459_70cf10f9","line":47,"range":{"start_line":40,"start_character":8,"end_line":47,"end_character":55},"updated":"2023-07-31 01:23:12.000000000","message":"These should be the main point of the fix, right?","commit_id":"432315978607730fea329269722af1499473ebea"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"2beed0cb4a10517ae404cbe77877152e38913173","unresolved":true,"context_lines":[{"line_number":37,"context_line":"            self, patch_int_ofport, arp_responder_enabled, dvr_enabled):"},{"line_number":38,"context_line":"        (dp, ofp, ofpp) \u003d self._get_dp()"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"        if not dvr_enabled:"},{"line_number":41,"context_line":"            # Table 0 (default) will sort incoming traffic depending on in_port"},{"line_number":42,"context_line":"            # This table is needed only for non-dvr environment because"},{"line_number":43,"context_line":"            # OVSDVRProcessMixin overwrites this flow in its"},{"line_number":44,"context_line":"            # install_dvr_process() method."},{"line_number":45,"context_line":"            self.install_goto(dest_table_id\u003dconstants.PATCH_LV_TO_TUN,"},{"line_number":46,"context_line":"                              priority\u003d1,"},{"line_number":47,"context_line":"                              in_port\u003dpatch_int_ofport)"},{"line_number":48,"context_line":""},{"line_number":49,"context_line":"        self.install_drop()  # default drop"},{"line_number":50,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"a1ea6f97_f6169e4b","line":47,"range":{"start_line":40,"start_character":8,"end_line":47,"end_character":55},"in_reply_to":"87874459_70cf10f9","updated":"2023-08-01 12:18:26.000000000","message":"Yes, there are two flows installed with the same match. This particular flow is overwritten by DVR tunneling specific flows by the DVR code. If this flow exists, there is a loop in the network.","commit_id":"432315978607730fea329269722af1499473ebea"}],"neutron/plugins/ml2/drivers/openvswitch/agent/ovs_dvr_neutron_agent.py":[{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"09e56ffefb80f0ca2c5bec508b13e66333556259","unresolved":true,"context_lines":[{"line_number":264,"context_line":"        if not self.enable_tunneling:"},{"line_number":265,"context_line":"            return"},{"line_number":266,"context_line":""},{"line_number":267,"context_line":"        self._setup_dvr_flows_on_tun_br(self.tun_br, self.patch_int_ofport)"},{"line_number":268,"context_line":""},{"line_number":269,"context_line":"    @staticmethod"},{"line_number":270,"context_line":"    def _setup_dvr_flows_on_tun_br(tun_br, patch_int_ofport):"},{"line_number":271,"context_line":"        tun_br.install_goto(dest_table_id\u003dovs_constants.DVR_PROCESS,"},{"line_number":272,"context_line":"                            priority\u003d1,"},{"line_number":273,"context_line":"                            in_port\u003dpatch_int_ofport)"},{"line_number":274,"context_line":""},{"line_number":275,"context_line":"        # table-miss should be sent to learning table"},{"line_number":276,"context_line":"        tun_br.install_goto(table_id\u003dovs_constants.DVR_NOT_LEARN,"},{"line_number":277,"context_line":"                            dest_table_id\u003dovs_constants.LEARN_FROM_TUN)"},{"line_number":278,"context_line":""},{"line_number":279,"context_line":"        tun_br.install_goto(table_id\u003dovs_constants.DVR_PROCESS,"},{"line_number":280,"context_line":"                            dest_table_id\u003dovs_constants.PATCH_LV_TO_TUN)"},{"line_number":281,"context_line":""},{"line_number":282,"context_line":"    def setup_dvr_flows_on_phys_br(self, bridge_mappings\u003dNone):"},{"line_number":283,"context_line":"        \u0027\u0027\u0027Setup up initial dvr flows into br-phys\u0027\u0027\u0027"}],"source_content_type":"text/x-python","patch_set":2,"id":"d34ff9b5_9059025b","line":280,"range":{"start_line":267,"start_character":0,"end_line":280,"end_character":72},"updated":"2023-07-31 01:23:12.000000000","message":"This new function is used by test cases only.","commit_id":"432315978607730fea329269722af1499473ebea"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"2beed0cb4a10517ae404cbe77877152e38913173","unresolved":true,"context_lines":[{"line_number":264,"context_line":"        if not self.enable_tunneling:"},{"line_number":265,"context_line":"            return"},{"line_number":266,"context_line":""},{"line_number":267,"context_line":"        self._setup_dvr_flows_on_tun_br(self.tun_br, self.patch_int_ofport)"},{"line_number":268,"context_line":""},{"line_number":269,"context_line":"    @staticmethod"},{"line_number":270,"context_line":"    def _setup_dvr_flows_on_tun_br(tun_br, patch_int_ofport):"},{"line_number":271,"context_line":"        tun_br.install_goto(dest_table_id\u003dovs_constants.DVR_PROCESS,"},{"line_number":272,"context_line":"                            priority\u003d1,"},{"line_number":273,"context_line":"                            in_port\u003dpatch_int_ofport)"},{"line_number":274,"context_line":""},{"line_number":275,"context_line":"        # table-miss should be sent to learning table"},{"line_number":276,"context_line":"        tun_br.install_goto(table_id\u003dovs_constants.DVR_NOT_LEARN,"},{"line_number":277,"context_line":"                            dest_table_id\u003dovs_constants.LEARN_FROM_TUN)"},{"line_number":278,"context_line":""},{"line_number":279,"context_line":"        tun_br.install_goto(table_id\u003dovs_constants.DVR_PROCESS,"},{"line_number":280,"context_line":"                            dest_table_id\u003dovs_constants.PATCH_LV_TO_TUN)"},{"line_number":281,"context_line":""},{"line_number":282,"context_line":"    def setup_dvr_flows_on_phys_br(self, bridge_mappings\u003dNone):"},{"line_number":283,"context_line":"        \u0027\u0027\u0027Setup up initial dvr flows into br-phys\u0027\u0027\u0027"}],"source_content_type":"text/x-python","patch_set":2,"id":"932a493b_a64c963a","line":280,"range":{"start_line":267,"start_character":0,"end_line":280,"end_character":72},"in_reply_to":"d34ff9b5_9059025b","updated":"2023-08-01 12:18:26.000000000","message":"Yes, commented in the test case about the reason of this breakdown.","commit_id":"432315978607730fea329269722af1499473ebea"}],"neutron/tests/functional/agent/test_ovs_flows.py":[{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"a2198bea54d46686efa57c4d3462faafdb997275","unresolved":false,"context_lines":[{"line_number":26,"context_line":"from neutron.common import utils as common_utils"},{"line_number":27,"context_line":"from neutron.plugins.ml2.drivers.openvswitch.agent \\"},{"line_number":28,"context_line":"    import ovs_neutron_agent as ovsagt"},{"line_number":29,"context_line":"from neutron.plugins.ml2.drivers.openvswitch.agent \\"},{"line_number":30,"context_line":"    import ovs_dvr_neutron_agent as ovsdvragt"},{"line_number":31,"context_line":"from neutron.tests.common import base as common_base"},{"line_number":32,"context_line":"from neutron.tests.common import helpers"}],"source_content_type":"text/x-python","patch_set":1,"id":"9ac7a63b_3938aaea","line":29,"in_reply_to":"0dc003b3_0c7a0a1d","updated":"2023-07-27 17:37:28.000000000","message":"\u003e pep8: H306: imports not in alphabetical order (neutron.plugins.ml2.drivers.openvswitch.agent.ovs_neutron_agent, neutron.plugins.ml2.drivers.openvswitch.agent.ovs_dvr_neutron_agent)\n\nDone","commit_id":"cdb89c33c083bab69b74a2832ae98e6ffbf8e26e"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"09e56ffefb80f0ca2c5bec508b13e66333556259","unresolved":true,"context_lines":[{"line_number":338,"context_line":"            common_utils.get_rand_device_name("},{"line_number":339,"context_line":"                prefix\u003dcfg.CONF.OVS.int_peer_patch_port))"},{"line_number":340,"context_line":"        self.br_tun.setup_default_table(self.tun_p, True, dvr_enabled)"},{"line_number":341,"context_line":"        ovsdvragt.OVSDVRNeutronAgent._setup_dvr_flows_on_tun_br(self.br_tun,"},{"line_number":342,"context_line":"                                                                self.tun_p)"},{"line_number":343,"context_line":""},{"line_number":344,"context_line":"    def test_provision_local_vlan(self):"}],"source_content_type":"text/x-python","patch_set":2,"id":"686e617e_be7f3928","line":341,"range":{"start_line":341,"start_character":37,"end_line":341,"end_character":63},"updated":"2023-07-31 01:23:12.000000000","message":"Why this call is needed for these test cases?","commit_id":"432315978607730fea329269722af1499473ebea"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"cda561d7dc3134bad08d43b92a3f651e6f877049","unresolved":true,"context_lines":[{"line_number":338,"context_line":"            common_utils.get_rand_device_name("},{"line_number":339,"context_line":"                prefix\u003dcfg.CONF.OVS.int_peer_patch_port))"},{"line_number":340,"context_line":"        self.br_tun.setup_default_table(self.tun_p, True, dvr_enabled)"},{"line_number":341,"context_line":"        ovsdvragt.OVSDVRNeutronAgent._setup_dvr_flows_on_tun_br(self.br_tun,"},{"line_number":342,"context_line":"                                                                self.tun_p)"},{"line_number":343,"context_line":""},{"line_number":344,"context_line":"    def test_provision_local_vlan(self):"}],"source_content_type":"text/x-python","patch_set":2,"id":"905d661b_e2d2957c","line":341,"range":{"start_line":341,"start_character":37,"end_line":341,"end_character":63},"in_reply_to":"686e617e_be7f3928","updated":"2023-07-31 16:24:51.000000000","message":"The tests were wrong and didn\u0027t install these flows. I had to strip it out of the agent class because of the bad composition of the code. The test_ovs_flows is not testing any module but simulates what agents install without actually instantiating the agents. The DVR agent always installs these flows but the test_ovs_flows doesn\u0027t account with that.\n\nIn order to cover the change with the tests, I had to make sure the change related agent flows are installed. I hope it answers your question, feel free to ask for more details.","commit_id":"432315978607730fea329269722af1499473ebea"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"2beed0cb4a10517ae404cbe77877152e38913173","unresolved":true,"context_lines":[{"line_number":338,"context_line":"            common_utils.get_rand_device_name("},{"line_number":339,"context_line":"                prefix\u003dcfg.CONF.OVS.int_peer_patch_port))"},{"line_number":340,"context_line":"        self.br_tun.setup_default_table(self.tun_p, True, dvr_enabled)"},{"line_number":341,"context_line":"        ovsdvragt.OVSDVRNeutronAgent._setup_dvr_flows_on_tun_br(self.br_tun,"},{"line_number":342,"context_line":"                                                                self.tun_p)"},{"line_number":343,"context_line":""},{"line_number":344,"context_line":"    def test_provision_local_vlan(self):"}],"source_content_type":"text/x-python","patch_set":2,"id":"616d8c5a_0d929024","line":341,"range":{"start_line":341,"start_character":37,"end_line":341,"end_character":63},"in_reply_to":"7a513c58_353661c2","updated":"2023-08-01 12:18:26.000000000","message":"Yes, these flows send packets from table 1 to table 2. The problematic flow in question is in table 2. Without these flows we don\u0027t have DVR for tunneling. I agree the flow testing can be better but I don\u0027t think that\u0027s in a scope of this patch.","commit_id":"432315978607730fea329269722af1499473ebea"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"7c16f1cae3eefe106796095f38fe1d236a249f39","unresolved":true,"context_lines":[{"line_number":338,"context_line":"            common_utils.get_rand_device_name("},{"line_number":339,"context_line":"                prefix\u003dcfg.CONF.OVS.int_peer_patch_port))"},{"line_number":340,"context_line":"        self.br_tun.setup_default_table(self.tun_p, True, dvr_enabled)"},{"line_number":341,"context_line":"        ovsdvragt.OVSDVRNeutronAgent._setup_dvr_flows_on_tun_br(self.br_tun,"},{"line_number":342,"context_line":"                                                                self.tun_p)"},{"line_number":343,"context_line":""},{"line_number":344,"context_line":"    def test_provision_local_vlan(self):"}],"source_content_type":"text/x-python","patch_set":2,"id":"7a513c58_353661c2","line":341,"range":{"start_line":341,"start_character":37,"end_line":341,"end_character":63},"in_reply_to":"905d661b_e2d2957c","updated":"2023-08-01 00:49:57.000000000","message":"OK, so these flows will be hit during the following trace? maybe we should verify the trace output flows with more details.","commit_id":"432315978607730fea329269722af1499473ebea"}],"neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_tunnel.py":[{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"7c16f1cae3eefe106796095f38fe1d236a249f39","unresolved":false,"context_lines":[{"line_number":189,"context_line":"        self._define_expected_calls()"},{"line_number":190,"context_line":""},{"line_number":191,"context_line":"    def _define_expected_calls("},{"line_number":192,"context_line":"            self, arp_responder\u003dFalse, igmp_snooping\u003dFalse, dvr_enabled\u003dFalse):"},{"line_number":193,"context_line":"        self.mock_int_bridge_cls_expected \u003d ["},{"line_number":194,"context_line":"            mock.call(self.INT_BRIDGE,"},{"line_number":195,"context_line":"                      datapath_type\u003dmock.ANY),"}],"source_content_type":"text/x-python","patch_set":2,"id":"c645cbe7_46cf2f58","line":192,"range":{"start_line":192,"start_character":60,"end_line":192,"end_character":77},"updated":"2023-08-01 00:49:57.000000000","message":"These is no case to call \"_define_expected_calls\" with dvr_enabled\u003dTrue. So this change seems not necessary.","commit_id":"432315978607730fea329269722af1499473ebea"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"2beed0cb4a10517ae404cbe77877152e38913173","unresolved":false,"context_lines":[{"line_number":189,"context_line":"        self._define_expected_calls()"},{"line_number":190,"context_line":""},{"line_number":191,"context_line":"    def _define_expected_calls("},{"line_number":192,"context_line":"            self, arp_responder\u003dFalse, igmp_snooping\u003dFalse, dvr_enabled\u003dFalse):"},{"line_number":193,"context_line":"        self.mock_int_bridge_cls_expected \u003d ["},{"line_number":194,"context_line":"            mock.call(self.INT_BRIDGE,"},{"line_number":195,"context_line":"                      datapath_type\u003dmock.ANY),"}],"source_content_type":"text/x-python","patch_set":2,"id":"0b16dc03_8aaeabc8","line":192,"range":{"start_line":192,"start_character":60,"end_line":192,"end_character":77},"in_reply_to":"c645cbe7_46cf2f58","updated":"2023-08-01 12:18:26.000000000","message":"Done","commit_id":"432315978607730fea329269722af1499473ebea"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"7c16f1cae3eefe106796095f38fe1d236a249f39","unresolved":false,"context_lines":[{"line_number":270,"context_line":""},{"line_number":271,"context_line":"        self.mock_tun_bridge_expected +\u003d ["},{"line_number":272,"context_line":"            mock.call.setup_default_table("},{"line_number":273,"context_line":"                self.INT_OFPORT, arp_responder, dvr_enabled),"},{"line_number":274,"context_line":"        ]"},{"line_number":275,"context_line":""},{"line_number":276,"context_line":"        self.ipdevice_expected \u003d []"}],"source_content_type":"text/x-python","patch_set":2,"id":"bad67251_ca0af448","line":273,"range":{"start_line":273,"start_character":48,"end_line":273,"end_character":59},"updated":"2023-08-01 00:49:57.000000000","message":"directly adding False to the param list is enough?\n\n            mock.call.setup_default_table(\n                self.INT_OFPORT, arp_responder, False),","commit_id":"432315978607730fea329269722af1499473ebea"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"2beed0cb4a10517ae404cbe77877152e38913173","unresolved":false,"context_lines":[{"line_number":270,"context_line":""},{"line_number":271,"context_line":"        self.mock_tun_bridge_expected +\u003d ["},{"line_number":272,"context_line":"            mock.call.setup_default_table("},{"line_number":273,"context_line":"                self.INT_OFPORT, arp_responder, dvr_enabled),"},{"line_number":274,"context_line":"        ]"},{"line_number":275,"context_line":""},{"line_number":276,"context_line":"        self.ipdevice_expected \u003d []"}],"source_content_type":"text/x-python","patch_set":2,"id":"840a26a9_82f49367","line":273,"range":{"start_line":273,"start_character":48,"end_line":273,"end_character":59},"in_reply_to":"bad67251_ca0af448","updated":"2023-08-01 12:18:26.000000000","message":"Done","commit_id":"432315978607730fea329269722af1499473ebea"}]}