)]}'
{"neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py":[{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"0dce727ddeb90c321cde6fbefed810ae6e34fe56","unresolved":false,"context_lines":[{"line_number":1551,"context_line":"            self.int_ofports[physical_network] \u003d int_ofport"},{"line_number":1552,"context_line":"            self.phys_ofports[physical_network] \u003d phys_ofport"},{"line_number":1553,"context_line":""},{"line_number":1554,"context_line":"            self.int_br.drop_port(in_port\u003dint_ofport)"},{"line_number":1555,"context_line":"            if not self.enable_distributed_routing:"},{"line_number":1556,"context_line":"                br.drop_port(in_port\u003dphys_ofport)"},{"line_number":1557,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"bf51134e_2515dc49","line":1554,"updated":"2020-07-14 05:37:22.000000000","message":"I think this deserves a comment","commit_id":"b41e0dad552bb3b14f5267d699615ca16f9b68e8"},{"author":{"_account_id":8313,"name":"Lajos Katona","display_name":"lajoskatona","email":"katonalala@gmail.com","username":"elajkat","status":"Ericsson Software Technology"},"change_message_id":"8d0a08319921702a1abb9af3c24ec0ef472624f5","unresolved":false,"context_lines":[{"line_number":1551,"context_line":"            self.int_ofports[physical_network] \u003d int_ofport"},{"line_number":1552,"context_line":"            self.phys_ofports[physical_network] \u003d phys_ofport"},{"line_number":1553,"context_line":""},{"line_number":1554,"context_line":"            self.int_br.drop_port(in_port\u003dint_ofport)"},{"line_number":1555,"context_line":"            if not self.enable_distributed_routing:"},{"line_number":1556,"context_line":"                br.drop_port(in_port\u003dphys_ofport)"},{"line_number":1557,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"bf51134e_2d42e20f","line":1554,"in_reply_to":"bf51134e_2515dc49","updated":"2020-07-14 08:48:48.000000000","message":"+1","commit_id":"b41e0dad552bb3b14f5267d699615ca16f9b68e8"},{"author":{"_account_id":2733,"name":"Darragh O\u0027Reilly","email":"doreilly@suse.com","username":"darragh-oreilly"},"change_message_id":"513ac2216b24ce6a4785b8eb368f580b8615a0c7","unresolved":false,"context_lines":[{"line_number":1551,"context_line":"            self.int_ofports[physical_network] \u003d int_ofport"},{"line_number":1552,"context_line":"            self.phys_ofports[physical_network] \u003d phys_ofport"},{"line_number":1553,"context_line":""},{"line_number":1554,"context_line":"            self.int_br.drop_port(in_port\u003dint_ofport)"},{"line_number":1555,"context_line":"            if not self.enable_distributed_routing:"},{"line_number":1556,"context_line":"                br.drop_port(in_port\u003dphys_ofport)"},{"line_number":1557,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"bf51134e_8d784ec4","line":1554,"in_reply_to":"bf51134e_2d42e20f","updated":"2020-07-14 09:34:06.000000000","message":"Done","commit_id":"b41e0dad552bb3b14f5267d699615ca16f9b68e8"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"295e8499e3f3ce69a71c07df4841d40f392d0a14","unresolved":false,"context_lines":[{"line_number":1572,"context_line":"                # bridge map settings, these tenant network traffic can also be"},{"line_number":1573,"context_line":"                # blocked by the following drop flows."},{"line_number":1574,"context_line":"                # block all untranslated traffic between bridges"},{"line_number":1575,"context_line":"                self.int_br.drop_port(in_port\u003dint_ofport)"},{"line_number":1576,"context_line":"                br.drop_port(in_port\u003dphys_ofport)"},{"line_number":1577,"context_line":""},{"line_number":1578,"context_line":"            if self.use_veth_interconnection:"},{"line_number":1579,"context_line":"                # enable veth to pass traffic"}],"source_content_type":"text/x-python","patch_set":2,"id":"bf51134e_ed674a24","side":"PARENT","line":1576,"range":{"start_line":1575,"start_character":15,"end_line":1576,"end_character":49},"updated":"2020-07-14 09:47:39.000000000","message":"OK, maybe I should insist on the approch of changing the priority of the dvr related drop flows [1].\n\nCloud you give a try of test in that way? Increse the flow priority of [2].\n\n[1] https://review.opendev.org/#/c/733568/5/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py@1592\n[2] https://review.opendev.org/#/c/733568/5/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_dvr_neutron_agent.py@248","commit_id":"73557abefcba1c6ce0cef709d1082674c0217485"},{"author":{"_account_id":2733,"name":"Darragh O\u0027Reilly","email":"doreilly@suse.com","username":"darragh-oreilly"},"change_message_id":"45632ff91e4f240889fdfd77a275e56c81d58202","unresolved":false,"context_lines":[{"line_number":1572,"context_line":"                # bridge map settings, these tenant network traffic can also be"},{"line_number":1573,"context_line":"                # blocked by the following drop flows."},{"line_number":1574,"context_line":"                # block all untranslated traffic between bridges"},{"line_number":1575,"context_line":"                self.int_br.drop_port(in_port\u003dint_ofport)"},{"line_number":1576,"context_line":"                br.drop_port(in_port\u003dphys_ofport)"},{"line_number":1577,"context_line":""},{"line_number":1578,"context_line":"            if self.use_veth_interconnection:"},{"line_number":1579,"context_line":"                # enable veth to pass traffic"}],"source_content_type":"text/x-python","patch_set":2,"id":"bf51134e_50b6c98b","side":"PARENT","line":1576,"range":{"start_line":1575,"start_character":15,"end_line":1576,"end_character":49},"in_reply_to":"bf51134e_ed674a24","updated":"2020-07-14 10:53:43.000000000","message":"Hi LIU Yulong,\n\nI don\u0027t see the need to increase the priority of the goto. The drop flow should only exist in table 0 when DVR is not enabled. With DVR the drop flow is in table 2 instead.\nhttps://github.com/openstack/neutron/blob/73557abefcba1c6ce0cef709d1082674c0217485/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_dvr_neutron_agent.py#L267-L270\n\nExample with DVR enabled:\n\n# ovs-ofctl dump-flows br-physnet1 --no-stats --names | cut -d\u0027,\u0027 -f2-\n priority\u003d2,in_port\u003d\"phy-br-physnet1\" actions\u003dresubmit(,1)\n priority\u003d0 actions\u003dNORMAL\n priority\u003d1 actions\u003dresubmit(,3)\n table\u003d1, priority\u003d0 actions\u003dresubmit(,2)\n table\u003d2, priority\u003d4,in_port\u003d\"phy-br-physnet1\",dl_vlan\u003d6 actions\u003dmod_vlan_vid:100,NORMAL\n table\u003d2, priority\u003d2,in_port\u003d\"phy-br-physnet1\" actions\u003ddrop\n table\u003d3, priority\u003d2,dl_src\u003dfa:16:3f:a8:f9:73 actions\u003doutput:\"phy-br-physnet1\"\n table\u003d3, priority\u003d1 actions\u003dNORMAL\n\nDVR disabled:\n priority\u003d4,in_port\u003d\"phy-br-physnet1\",dl_vlan\u003d6 actions\u003dmod_vlan_vid:100,NORMAL\n priority\u003d2,in_port\u003d\"phy-br-physnet1\" actions\u003ddrop\n priority\u003d0 actions\u003dNORMAL","commit_id":"73557abefcba1c6ce0cef709d1082674c0217485"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"c22e2b172488787afefa23ba362b4a07589253d3","unresolved":false,"context_lines":[{"line_number":1555,"context_line":"            # priority flow to set a local vlan. This prevents these stray"},{"line_number":1556,"context_line":"            # packets from being forwarded to other physical bridges which"},{"line_number":1557,"context_line":"            # could cause a network loop in the physical network."},{"line_number":1558,"context_line":"            self.int_br.drop_port(in_port\u003dint_ofport)"},{"line_number":1559,"context_line":""},{"line_number":1560,"context_line":"            if not self.enable_distributed_routing:"},{"line_number":1561,"context_line":"                br.drop_port(in_port\u003dphys_ofport)"}],"source_content_type":"text/x-python","patch_set":2,"id":"bf51134e_e899f16c","line":1558,"updated":"2020-07-14 16:57:59.000000000","message":"This is, in practice, like reverting 90212b12.\n\nAs commented in the previous patch (the revert), why don\u0027t you explore the option of tagging the traffic coming into the int-br from the phys-brs?\n\nIn case the OVS has been restarted (no flows remain from previous executions), the traffic from the physical bridges can be tagged in the local switching table (0), for example writing a register.\n\nThen, in the physical bridges, you can add rules to drop this tagged traffic. That will prevent what you describe in the bug.","commit_id":"c1a77ef8b74bb9b5abbc5cb03fb3201383122eb8"},{"author":{"_account_id":2733,"name":"Darragh O\u0027Reilly","email":"doreilly@suse.com","username":"darragh-oreilly"},"change_message_id":"f56f10587320dbba00d3f50acc51bcd04f6856ee","unresolved":false,"context_lines":[{"line_number":1555,"context_line":"            # priority flow to set a local vlan. This prevents these stray"},{"line_number":1556,"context_line":"            # packets from being forwarded to other physical bridges which"},{"line_number":1557,"context_line":"            # could cause a network loop in the physical network."},{"line_number":1558,"context_line":"            self.int_br.drop_port(in_port\u003dint_ofport)"},{"line_number":1559,"context_line":""},{"line_number":1560,"context_line":"            if not self.enable_distributed_routing:"},{"line_number":1561,"context_line":"                br.drop_port(in_port\u003dphys_ofport)"}],"source_content_type":"text/x-python","patch_set":2,"id":"bf51134e_cd3de9bd","line":1558,"in_reply_to":"bf51134e_0094ea89","updated":"2020-07-17 10:03:26.000000000","message":"\u003e The traffic will be blocked, in practice is a revert\n\nI tested this patch with and without dvr, and there is no traffic interuption on agent restart. \nI know the test is good because at 10 pings/sec and commit \"Do not block connection between br-int and br-phys on startup\" reverted, I always see drops.\nThis patch does not cause a regression of bug 1869808.","commit_id":"c1a77ef8b74bb9b5abbc5cb03fb3201383122eb8"},{"author":{"_account_id":2733,"name":"Darragh O\u0027Reilly","email":"doreilly@suse.com","username":"darragh-oreilly"},"change_message_id":"9eff322e84db54004251f2fd6c9b9bd9450dd0d0","unresolved":false,"context_lines":[{"line_number":1555,"context_line":"            # priority flow to set a local vlan. This prevents these stray"},{"line_number":1556,"context_line":"            # packets from being forwarded to other physical bridges which"},{"line_number":1557,"context_line":"            # could cause a network loop in the physical network."},{"line_number":1558,"context_line":"            self.int_br.drop_port(in_port\u003dint_ofport)"},{"line_number":1559,"context_line":""},{"line_number":1560,"context_line":"            if not self.enable_distributed_routing:"},{"line_number":1561,"context_line":"                br.drop_port(in_port\u003dphys_ofport)"}],"source_content_type":"text/x-python","patch_set":2,"id":"bf51134e_e5fa93b7","line":1558,"in_reply_to":"bf51134e_0094ea89","updated":"2020-07-16 14:15:38.000000000","message":"\u003e The traffic will be blocked, in practice is a revert\n\nRodolfo, please explain to me how exactly this flow on br-int could possibility cause a traffic interruption on agent restart.","commit_id":"c1a77ef8b74bb9b5abbc5cb03fb3201383122eb8"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"fd9b25431a990a137e87f66ea2729b8c7108f127","unresolved":false,"context_lines":[{"line_number":1555,"context_line":"            # priority flow to set a local vlan. This prevents these stray"},{"line_number":1556,"context_line":"            # packets from being forwarded to other physical bridges which"},{"line_number":1557,"context_line":"            # could cause a network loop in the physical network."},{"line_number":1558,"context_line":"            self.int_br.drop_port(in_port\u003dint_ofport)"},{"line_number":1559,"context_line":""},{"line_number":1560,"context_line":"            if not self.enable_distributed_routing:"},{"line_number":1561,"context_line":"                br.drop_port(in_port\u003dphys_ofport)"}],"source_content_type":"text/x-python","patch_set":2,"id":"bf51134e_0094ea89","line":1558,"in_reply_to":"bf51134e_687961c3","updated":"2020-07-15 14:43:44.000000000","message":"The traffic will be blocked, in practice is a revert\n\nRight, the registers are not passed when using normal action.\n\nJust one last option: did you try to configure STP in all bridges? You can do this manually and test it. Is it possible for you?","commit_id":"c1a77ef8b74bb9b5abbc5cb03fb3201383122eb8"},{"author":{"_account_id":2733,"name":"Darragh O\u0027Reilly","email":"doreilly@suse.com","username":"darragh-oreilly"},"change_message_id":"c3c46e6551c0644865709900e9fb9e1cb5e0b80c","unresolved":false,"context_lines":[{"line_number":1555,"context_line":"            # priority flow to set a local vlan. This prevents these stray"},{"line_number":1556,"context_line":"            # packets from being forwarded to other physical bridges which"},{"line_number":1557,"context_line":"            # could cause a network loop in the physical network."},{"line_number":1558,"context_line":"            self.int_br.drop_port(in_port\u003dint_ofport)"},{"line_number":1559,"context_line":""},{"line_number":1560,"context_line":"            if not self.enable_distributed_routing:"},{"line_number":1561,"context_line":"                br.drop_port(in_port\u003dphys_ofport)"}],"source_content_type":"text/x-python","patch_set":2,"id":"bf51134e_ff1b30c7","line":1558,"in_reply_to":"bf51134e_bfc53840","updated":"2020-07-17 11:28:20.000000000","message":"\u003e (This is just my preference) the DVR traffic should be handled only\n \u003e in DVRAgent; this condition \"enable_distributed_routing\" should not\n \u003e be here.\n\nfair enough. But my patch did not add that condition. It was already in master.","commit_id":"c1a77ef8b74bb9b5abbc5cb03fb3201383122eb8"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"22b80c3f4e4c65104f01fdad8dc927e5e3258e11","unresolved":false,"context_lines":[{"line_number":1555,"context_line":"            # priority flow to set a local vlan. This prevents these stray"},{"line_number":1556,"context_line":"            # packets from being forwarded to other physical bridges which"},{"line_number":1557,"context_line":"            # could cause a network loop in the physical network."},{"line_number":1558,"context_line":"            self.int_br.drop_port(in_port\u003dint_ofport)"},{"line_number":1559,"context_line":""},{"line_number":1560,"context_line":"            if not self.enable_distributed_routing:"},{"line_number":1561,"context_line":"                br.drop_port(in_port\u003dphys_ofport)"}],"source_content_type":"text/x-python","patch_set":2,"id":"bf51134e_bfc53840","line":1558,"in_reply_to":"bf51134e_bff258e2","updated":"2020-07-17 11:15:41.000000000","message":"This could work, actually.\n\n(This is just my preference) the DVR traffic should be handled only in DVRAgent; this condition \"enable_distributed_routing\" should not be here.\n\nAnyway, you are right, this patch does not overwrite the phy br rule. I\u0027ll remove the -1, but I prefer my solution.","commit_id":"c1a77ef8b74bb9b5abbc5cb03fb3201383122eb8"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"d16f895b683f35328c5e914777075710f2495b68","unresolved":false,"context_lines":[{"line_number":1555,"context_line":"            # priority flow to set a local vlan. This prevents these stray"},{"line_number":1556,"context_line":"            # packets from being forwarded to other physical bridges which"},{"line_number":1557,"context_line":"            # could cause a network loop in the physical network."},{"line_number":1558,"context_line":"            self.int_br.drop_port(in_port\u003dint_ofport)"},{"line_number":1559,"context_line":""},{"line_number":1560,"context_line":"            if not self.enable_distributed_routing:"},{"line_number":1561,"context_line":"                br.drop_port(in_port\u003dphys_ofport)"}],"source_content_type":"text/x-python","patch_set":2,"id":"bf51134e_cdcde95f","line":1558,"in_reply_to":"bf51134e_cd3de9bd","updated":"2020-07-17 10:30:57.000000000","message":"This drop flow can cause traffic interruption with DVR in a live environment (please, remember this matrix of DVR/non-DVR - live traffic/flows removed).\n\nAs said, with DVR and live traffic, this drop flow will overwrite [1]. This rule takes the traffic coming into the physical bridge and submit it to DVR_PROCESS_PHYSICAL (table 1). Other flows will process the traffic from this point.\n\nIf, as commented in [2], the RPC channel is down or unresponsive, between the drop flow (applied during the physical bridge initialization) and [1] (applied during the DVR class initialization), there will be a period of time where the traffic coming into the phys bridge will be dropped.\n\nInstead of this, for the case of DVR with live traffic, instead of setting prio 2 in [1], Liu and I recommended to set prio 3. That will work for all cases [3].\n\n\n[1]https://github.com/openstack/neutron/blob/d28654a0c740844ab726d557480ebb3fd3ac66c7/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_dvr_neutron_agent.py#L256-L259\n[2]https://bugs.launchpad.net/neutron/+bug/1869808\n[3]https://review.opendev.org/#/c/741444/","commit_id":"c1a77ef8b74bb9b5abbc5cb03fb3201383122eb8"},{"author":{"_account_id":2733,"name":"Darragh O\u0027Reilly","email":"doreilly@suse.com","username":"darragh-oreilly"},"change_message_id":"2922e01ff247c95d9063b6a0f1d086f9867aba9a","unresolved":false,"context_lines":[{"line_number":1555,"context_line":"            # priority flow to set a local vlan. This prevents these stray"},{"line_number":1556,"context_line":"            # packets from being forwarded to other physical bridges which"},{"line_number":1557,"context_line":"            # could cause a network loop in the physical network."},{"line_number":1558,"context_line":"            self.int_br.drop_port(in_port\u003dint_ofport)"},{"line_number":1559,"context_line":""},{"line_number":1560,"context_line":"            if not self.enable_distributed_routing:"},{"line_number":1561,"context_line":"                br.drop_port(in_port\u003dphys_ofport)"}],"source_content_type":"text/x-python","patch_set":2,"id":"bf51134e_bff258e2","line":1558,"in_reply_to":"bf51134e_cdcde95f","updated":"2020-07-17 11:02:30.000000000","message":"\u003e As said, with DVR and live traffic, this drop flow will overwrite\n \u003e [1]. This rule takes the traffic coming into the physical bridge\n \u003e and submit it to DVR_PROCESS_PHYSICAL (table 1). Other flows will\n \u003e process the traffic from this point.\n \u003e \n\nL1558 in my commit is adding the drop flow to br-int. It will not overwrite the drop flow in [1] which is the physical bridge.\n\n[1]https://github.com/openstack/neutron/blob/d28654a0c740844ab726d557480ebb3fd3ac66c7/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_dvr_neutron_agent.py#L256-L259","commit_id":"c1a77ef8b74bb9b5abbc5cb03fb3201383122eb8"},{"author":{"_account_id":2733,"name":"Darragh O\u0027Reilly","email":"doreilly@suse.com","username":"darragh-oreilly"},"change_message_id":"d43b380d12e4f6a5ebc36151b992ddc8d88bc0e2","unresolved":false,"context_lines":[{"line_number":1555,"context_line":"            # priority flow to set a local vlan. This prevents these stray"},{"line_number":1556,"context_line":"            # packets from being forwarded to other physical bridges which"},{"line_number":1557,"context_line":"            # could cause a network loop in the physical network."},{"line_number":1558,"context_line":"            self.int_br.drop_port(in_port\u003dint_ofport)"},{"line_number":1559,"context_line":""},{"line_number":1560,"context_line":"            if not self.enable_distributed_routing:"},{"line_number":1561,"context_line":"                br.drop_port(in_port\u003dphys_ofport)"}],"source_content_type":"text/x-python","patch_set":2,"id":"bf51134e_687961c3","line":1558,"in_reply_to":"bf51134e_e899f16c","updated":"2020-07-14 17:23:01.000000000","message":"\u003e This is, in practice, like reverting 90212b12.\n\u003e \n\nIt\u0027s a semi revert. This flow is needed early on for dvr as well as non-dvr. IMO commit 90212b12 should not have changed it. I don\u0027t see how it could have caused bug 1869808 and there is no analysis or evidence to show that it did. The other flow on the physical bridge was the problem. \n\n\u003e As commented in the previous patch (the revert), why don\u0027t you\n\u003e explore the option of tagging the traffic coming into the int-br\n\u003e from the phys-brs?\n\u003e \n\nI did - registers don\u0027t work across bridges.","commit_id":"c1a77ef8b74bb9b5abbc5cb03fb3201383122eb8"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"d80a53d4781db805f0cfb5baaa5198c6799b7619","unresolved":false,"context_lines":[{"line_number":1555,"context_line":"            # priority flow to set a local vlan. This prevents these stray"},{"line_number":1556,"context_line":"            # packets from being forwarded to other physical bridges which"},{"line_number":1557,"context_line":"            # could cause a network loop in the physical network."},{"line_number":1558,"context_line":"            self.int_br.drop_port(in_port\u003dint_ofport)"},{"line_number":1559,"context_line":""},{"line_number":1560,"context_line":"            if not self.enable_distributed_routing:"},{"line_number":1561,"context_line":"                br.drop_port(in_port\u003dphys_ofport)"}],"source_content_type":"text/x-python","patch_set":2,"id":"bf51134e_df5c6c4e","line":1558,"in_reply_to":"bf51134e_ff1b30c7","updated":"2020-07-17 11:44:36.000000000","message":"Actually DVR agent is adding this drop in int-br too [1].\n\nAnd with this patch, you don\u0027t need manually to add the flow with prio 3 in the physical bridge (as my patch needs).\n\nHmmmm maybe because of this, this patch is less intrusive.\n\n[1]https://github.com/openstack/neutron/blob/d28654a0c740844ab726d557480ebb3fd3ac66c7/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_dvr_neutron_agent.py#L230-L234","commit_id":"c1a77ef8b74bb9b5abbc5cb03fb3201383122eb8"}]}