)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"7cb2a47356801072b2bee29553fd1f54fcb027c3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"450cc730_e490904e","updated":"2022-01-27 09:09:13.000000000","message":"Is there a check for \"port tag !\u003d DEAD_VLAN\" before adding a dead flow? Waiting for dead flow just to delete it does not seem an optimal way.. Also the reason for this race is better be investigated: ports are processed in a single thread, so maybe the order (priority) should be fixed here.","commit_id":"9c673cf48cc1dcfb8b15fb08cfd1c02a4ba1621e"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"884cb3a2c0c5d64c02dabc82e0c3d432c958e93d","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"cb5ff399_aa30bab5","in_reply_to":"29b3a18a_ba4aa5bf","updated":"2022-02-01 12:30:45.000000000","message":"\u003e Sorry for the slow response. I did test what you suggested. It works, but with caveats.\n\u003e \n\u003e I plugged two libvirt vms into br0 and set their ports like this:\n\u003e \n\u003e  for vm in vm0 vm1\n\u003e  do\n\u003e    ovs-vsctl clear Port \"$( virsh dumpxml \"$vm\" | xmlstarlet sel -t -v \u0027//interface[2]/target/@dev\u0027 )\" tag\n\u003e    ovs-vsctl set Port \"$( virsh dumpxml \"$vm\" | xmlstarlet sel -t -v \u0027//interface[2]/target/@dev\u0027 )\" vlan_mode\u003dtrunk\n\u003e    ovs-vsctl set Port \"$( virsh dumpxml \"$vm\" | xmlstarlet sel -t -v \u0027//interface[2]/target/@dev\u0027 )\" trunks\u003d4095\n\u003e  done\n\u003e \n\u003e In this test environment I only had the single default normal action flow:\n\u003e \n\u003e  # ovs-ofctl dump-flows br0\n\u003e   cookie\u003d0x0, duration\u003d345716.916s, table\u003d0, n_packets\u003d80851, n_bytes\u003d3441463, priority\u003d0 actions\u003dNORMAL\n\u003e \n\u003e In this case any vlan that\u0027s not vlan 4095 gets dropped as you said. Which I could also confirm like this:\n\u003e \n\u003e  # ovs-appctl ofproto/trace br0 in_port\u003d1,dl_vlan\u003d0\n\u003e  Flow: in_port\u003d1,dl_vlan\u003d0,dl_vlan_pcp\u003d0,vlan_tci1\u003d0x0000,dl_src\u003d00:00:00:00:00:00,dl_dst\u003d00:00:00:00:00:00,dl_type\u003d0x0000\n\u003e ·\n\u003e  bridge(\"br0\")\n\u003e  -------------\n\u003e   0. priority 0\n\u003e      NORMAL\n\u003e       \u003e\u003e\u003e\u003e dropping VLAN 0 packet received on port vnet1 not configured for trunking VLAN 0 \u003c\u003c\u003c\u003c\n\u003e       \u003e\u003e disallowed VLAN VID for this input port, dropping\n\u003e ·\n\u003e  Final flow: unchanged\n\u003e  Megaflow: recirc_id\u003d0,eth,in_port\u003d1,dl_vlan\u003d0,dl_src\u003d00:00:00:00:00:00,dl_dst\u003d00:00:00:00:00:00,dl_type\u003d0x0000\n\u003e  Datapath actions: drop\n\u003e \n\u003e But we have caveats:\n\u003e \n\u003e 1) If we implement the dead vlan this way, then all ports plugged into the dead vlan are able to communicate with each other on vlan 4095 (at least inside a compute host). vlan 4095 is a reserved vlan, so for example Linux does not allow a vlan subinterface with id 4095, but clearly if you can modify what\u0027s on the wire, you can use vlan 4095 as a valid vlan. ovs does not treat it in a special way. This already means that we must totally trust whatever is connected to a dead vlan port (all dhcp server and router implementations we allow). If they happen to have an independent vulnerability that may be combined with the fact, that they have a ready-made communication channel between each other crossing tenant separation.\n\u003e \n\nBut we still have default drop rule in br-int for packets with vlan 4095 - that should not allow communication over that vlan, right? (Though I\u0027m not sure how dhcp/router ports could even try to do this)\n\n\u003e 2) Keep in mind that the normal action flow(s) are always the last flows in the flow table. Anything connected to dead vlan ports may try subverting all flows before the normal action flow. For example flows handling traffic towards physical bridges. This can happen even outside vlan 4095.\n\nNot sure I got this. My vision is: new plugged dhcp/router port won\u0027t be able to send untagged packets or any tagged packets with vlan!\u003d4095 (because of configured trunks); on the other hand packets with vlan 4095 will be dropped by br-int.\nAm I missing something?","commit_id":"9c673cf48cc1dcfb8b15fb08cfd1c02a4ba1621e"},{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"working for Ericsson, UTC+1 (+DST)"},"change_message_id":"e17f1b963d7ff23b76691e0fef298a187c7049b1","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"93a7938d_fb095f5e","in_reply_to":"450cc730_e490904e","updated":"2022-01-27 10:25:05.000000000","message":"Please have a look at the previous change referred from the commit message.\n\nAt a high level, we had a security bug, we leaked traffic between ports plugged into the dead vlan. We had to install a flow (temporarily) for each of these ports to actually drop their traffic. But due to the the race condition described here (which we didn\u0027t realize was possible during the previous patch\u0027s review and testing until we hit some problems in the stable/victoria gate) the add-delete order may be wrong and then we leave the port dead.\n\nTechnically we add the dead vlan flow from dhcp/l3-agent, but we delete it from ovs-agent. So adding and deleting happens in two different processes. As soon as dhcp/l3-agents plug the port ovs-agent may start processing the same port. But dhcp/l3-agents also did not finish with the port when it was plugged, that\u0027s why the race is possible.\n\nI also think this is far from optimal and it may cause some slowdown in ovs-agent\u0027s port processing. But the previous patch was created to fix a bug with possible security implications, so I believe fixing that is more important than some performance loss. Ideally we could fix the security bug without any performance loss. Please let me know if you see a way to do that.\n\nRegarding the check you asked for: ovs_lib\u0027s replace_port() always puts the port into the dead vlan, so I think there\u0027s no need to further check it.","commit_id":"9c673cf48cc1dcfb8b15fb08cfd1c02a4ba1621e"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"3bf6768e0f61780ca6ddae67cbb00e4e2fbace2b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"38557304_4d1238f0","in_reply_to":"491429cd_66fb77b0","updated":"2022-01-31 12:53:24.000000000","message":"Sorry, in the last sentence it\u0027s not the VM, but the DHCP/Router ports as this is what bug is about","commit_id":"9c673cf48cc1dcfb8b15fb08cfd1c02a4ba1621e"},{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"working for Ericsson, UTC+1 (+DST)"},"change_message_id":"7580b61c2709322b8ff95099cc08e1a4bb85658e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"639cdf8d_73ebb8e1","in_reply_to":"491429cd_66fb77b0","updated":"2022-01-31 13:15:01.000000000","message":"That\u0027s an idea I did no get at the first time.\n\nBut is there such an invalid config for vlan_mode? A port in trunk mode always accepts untagged frames, doesn\u0027t it?\n\nLet\u0027s say I set the port like this:\n\n ovs-vsctl clear port \"$( virsh dumpxml vm0 | xmlstarlet sel -t -v \u0027//interface[2]/target/@dev\u0027 )\" tag\n ovs-vsctl set Port \"$( virsh dumpxml vm0 | xmlstarlet sel -t -v \u0027//interface[2]/target/@dev\u0027 )\" vlan_mode\u003dtrunk\n ovs-vsctl set port \"$( virsh dumpxml vm0 | xmlstarlet sel -t -v \u0027//interface[2]/target/@dev\u0027 )\" trunks\u003d4095 # I think trunks\u003d[] would be more restrictive.\n\nAnd I start sending untagged traffic on that port (ingress for the switch).\n\nThen I see those frames in the packet counter of the default actions\u003dNORMAL flow.\n\nI think that means we cannot prohibit the use of the untagged vlan on a trunk with this config.","commit_id":"9c673cf48cc1dcfb8b15fb08cfd1c02a4ba1621e"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"4c53b0371bb994ea855a8ecbccb5f65ccb56021f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"bee4eb5d_d5ffdf40","in_reply_to":"4de78ea2_a0cd6e06","updated":"2022-01-31 14:28:53.000000000","message":"I\u0027d still try to reproduce a bug with setting vlan_mode to trunk (plus trunks to 4095) or native-tagged. Not sure flow counter growing means that packet wasn\u0027t actually dropped at a later stage (during actual handling of NORMAL rule)","commit_id":"9c673cf48cc1dcfb8b15fb08cfd1c02a4ba1621e"},{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"working for Ericsson, UTC+1 (+DST)"},"change_message_id":"ce5a6d1313358ce17ec90daf14b8c45643b56f9a","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"ebdefdbb_be46e01d","in_reply_to":"62046c50_2c9bef24","updated":"2022-02-01 13:43:43.000000000","message":"\u003e Ah, got it. Still this is out of scope of the original bug, right? Also those ports normally don\u0027t do anything like that..\n\nNormally they don\u0027t.","commit_id":"9c673cf48cc1dcfb8b15fb08cfd1c02a4ba1621e"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"823aeca765127a8c2416ff8665019f0e8367188c","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"941b71f6_8e504cba","in_reply_to":"62046c50_2c9bef24","updated":"2022-02-01 13:43:40.000000000","message":"BTW this should also work with \"native-tagged\" vlan_mode - together with tag\u003d4095 that will be the same as with trunks. Thus only need to config single new port field (and clear later by ovs agent)","commit_id":"9c673cf48cc1dcfb8b15fb08cfd1c02a4ba1621e"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"720b4192dee6f270973b89f0aa3c410935836f12","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"65513d75_59ae98a9","in_reply_to":"639cdf8d_73ebb8e1","updated":"2022-01-31 13:30:33.000000000","message":"\u003e That\u0027s an idea I did no get at the first time.\n\u003e \n\u003e But is there such an invalid config for vlan_mode? A port in trunk mode always accepts untagged frames, doesn\u0027t it?\n\nAccording to [1] \"trunk: The packet is in the VLAN specified in its 802.1Q header, or in VLAN 0 if there is no 802.1Q header. The trunks column in the Port record lists the valid VLANs; if it is empty, all VLANs are valid.\"\n\nSo if we set trunks\u003d[4095] - only packets tagged with 4095 are \"valid\". In order to accept untagged packets I assume \"trunks\" should also contain \"0\".\n\nIt should be easier to just try it if have repro handy - it\u0027s a matter of 2 lines of code in replace_port() function.\n\n[1] https://docs.openvswitch.org/en/latest/ref/ovs-actions.7/?highlight\u003dovs-actions#the-ovs-normal-pipeline\n\n\u003e \n\u003e Let\u0027s say I set the port like this:\n\u003e \n\u003e  ovs-vsctl clear port \"$( virsh dumpxml vm0 | xmlstarlet sel -t -v \u0027//interface[2]/target/@dev\u0027 )\" tag\n\u003e  ovs-vsctl set Port \"$( virsh dumpxml vm0 | xmlstarlet sel -t -v \u0027//interface[2]/target/@dev\u0027 )\" vlan_mode\u003dtrunk\n\u003e  ovs-vsctl set port \"$( virsh dumpxml vm0 | xmlstarlet sel -t -v \u0027//interface[2]/target/@dev\u0027 )\" trunks\u003d4095 # I think trunks\u003d[] would be more restrictive.\n\n\"if it is empty, all VLANs are valid\" - so it\u0027s not more restrictive\n\n\u003e \n\u003e And I start sending untagged traffic on that port (ingress for the switch).\n\u003e \n\u003e Then I see those frames in the packet counter of the default actions\u003dNORMAL flow.\n\u003e \n\u003e I think that means we cannot prohibit the use of the untagged vlan on a trunk with this config.","commit_id":"9c673cf48cc1dcfb8b15fb08cfd1c02a4ba1621e"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"2a57ab8f5895dff89174bfe16afcf3d7fe49a32f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"cc14d607_cb18fd3e","in_reply_to":"65513d75_59ae98a9","updated":"2022-01-31 13:39:06.000000000","message":"\u003e \u003e That\u0027s an idea I did no get at the first time.\n\u003e \u003e \n\u003e \u003e But is there such an invalid config for vlan_mode? A port in trunk mode always accepts untagged frames, doesn\u0027t it?\n\u003e \n\u003e According to [1] \"trunk: The packet is in the VLAN specified in its 802.1Q header, or in VLAN 0 if there is no 802.1Q header. The trunks column in the Port record lists the valid VLANs; if it is empty, all VLANs are valid.\"\n\u003e \n\u003e So if we set trunks\u003d[4095] - only packets tagged with 4095 are \"valid\". In order to accept untagged packets I assume \"trunks\" should also contain \"0\".\n\u003e \n\u003e It should be easier to just try it if have repro handy - it\u0027s a matter of 2 lines of code in replace_port() function.\n\u003e \n\u003e [1] https://docs.openvswitch.org/en/latest/ref/ovs-actions.7/?highlight\u003dovs-actions#the-ovs-normal-pipeline\n\u003e \n\u003e \u003e \n\u003e \u003e Let\u0027s say I set the port like this:\n\u003e \u003e \n\u003e \u003e  ovs-vsctl clear port \"$( virsh dumpxml vm0 | xmlstarlet sel -t -v \u0027//interface[2]/target/@dev\u0027 )\" tag\n\u003e \u003e  ovs-vsctl set Port \"$( virsh dumpxml vm0 | xmlstarlet sel -t -v \u0027//interface[2]/target/@dev\u0027 )\" vlan_mode\u003dtrunk\n\u003e \u003e  ovs-vsctl set port \"$( virsh dumpxml vm0 | xmlstarlet sel -t -v \u0027//interface[2]/target/@dev\u0027 )\" trunks\u003d4095 # I think trunks\u003d[] would be more restrictive.\n\u003e \n\u003e \"if it is empty, all VLANs are valid\" - so it\u0027s not more restrictive\n\u003e \n\u003e \u003e \n\u003e \u003e And I start sending untagged traffic on that port (ingress for the switch).\n\u003e \u003e \n\u003e \u003e Then I see those frames in the packet counter of the default actions\u003dNORMAL flow.\n\u003e \u003e \n\u003e \u003e I think that means we cannot prohibit the use of the untagged vlan on a trunk with this config.\n\nWe need to drop packets _from_ the new port in the first place, right?","commit_id":"9c673cf48cc1dcfb8b15fb08cfd1c02a4ba1621e"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"d083de7d4c4f0132461f52a5caf00e46c09aef6b","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"aec69752_10aa1cb4","in_reply_to":"93a7938d_fb095f5e","updated":"2022-01-27 13:08:55.000000000","message":"Ah, I see, so the flow in question is added here:\nhttps://review.opendev.org/c/openstack/neutron/+/820897/15/neutron/agent/common/ovs_lib.py - which is used by OVSInterfaceDriver in l3/dhcp agents.\nAnd this kind of breaks separation of concerns, where all flow modifications should be handled by l2 agent, however I see this was done on purpose.\n\nReading through [1] I think maybe instead of inserting DEAD_VLAN_TAG with a flow rule we may try to play with vlan_mode setting of a port: like setting it native-tagged or native-untagged? Will such ports match the default dead-vlan-drop rule of br-int? Then OVS agent will reset vlan-mode to \"access\" unconditionally..\n\n[1] https://docs.openvswitch.org/en/latest/ref/ovs-actions.7/?highlight\u003dovs-actions#the-ovs-normal-pipeline","commit_id":"9c673cf48cc1dcfb8b15fb08cfd1c02a4ba1621e"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"9e5ae47484761d32ca6d16cf8b4e7427e3450cd1","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"491429cd_66fb77b0","in_reply_to":"9740620e_094254d5","updated":"2022-01-31 12:47:26.000000000","message":"Sorry, one more round, before I give up, please :) (just to make sure we\u0027re on the same page)\n\nThe idea is to plug a new port with an a priori \"invalid\" config for vlan_mode, so that all packets are dropped, no matter what openflow rules are configured further. IIUC this should resolve original bug with leaked packets. Then when OVS agent handles such a port it sets correct vlan_mode.\n\nSo if we set vlan_mode\u003dtrunk and trunks\u003d[4095] there seems to be no way the VM can send valid packets, and everything should be dropped. Did you try with these values?","commit_id":"9c673cf48cc1dcfb8b15fb08cfd1c02a4ba1621e"},{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"working for Ericsson, UTC+1 (+DST)"},"change_message_id":"f9f7a87783f3c103de34021193e25b36599362a3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"9740620e_094254d5","in_reply_to":"a563e800_da38e9db","updated":"2022-01-31 12:17:14.000000000","message":"Unfortunately all vlan_modes apply only to \"normal switching\" and not to the openflow pipeline. I guess the underlying reason is the optimization to modify the frames (ie. push the vlan tag) as late as possible.\n\nhttp://www.openvswitch.org/support/dist-docs/ovs-vswitchd.conf.db.5.html\n\n       The VLAN configuration in this column affects Open vSwitch only when it\n       is  doing  ``normal  switching.’’ It does not affect flows set up by an\n       OpenFlow controller, outside of the OpenFlow ``normal action.’’\n\nSome vlan_modes would also rely on the guest vm sending and/or receiving tagged frames. That we cannot expect from the guest OS.\n\nI also actually tested these with ovs and a libvirt vm plugged into it.\nHaving flows like this:\n\n # ovs-ofctl dump-flows br0\n cookie\u003d0x1, duration\u003d760.177s, table\u003d0, n_packets\u003d0, n_bytes\u003d0, priority\u003d65535,dl_vlan\u003d10 actions\u003ddrop\n cookie\u003d0x0, duration\u003d260534.938s, table\u003d0, n_packets\u003d970, n_bytes\u003d63938, priority\u003d0 actions\u003dNORMAL\n\nAnd watching the flow match packet counters while trying the below scenarios:\n\n ovs-vsctl set Port \"$( virsh dumpxml vm0 | xmlstarlet sel -t -v \u0027//interface[2]/target/@dev\u0027 )\" vlan_mode\u003daccess\n ovs-vsctl set port \"$( virsh dumpxml vm0 | xmlstarlet sel -t -v \u0027//interface[2]/target/@dev\u0027 )\" tag\u003d10\n dl_vlan\u003d10 does not match\n\n ovs-vsctl set Port \"$( virsh dumpxml vm0 | xmlstarlet sel -t -v \u0027//interface[2]/target/@dev\u0027 )\" vlan_mode\u003dnative-tagged\n ovs-vsctl set port \"$( virsh dumpxml vm0 | xmlstarlet sel -t -v \u0027//interface[2]/target/@dev\u0027 )\" tag\u003d10\n dl_vlan\u003d10 does not match\n\n ovs-vsctl set Port \"$( virsh dumpxml vm0 | xmlstarlet sel -t -v \u0027//interface[2]/target/@dev\u0027 )\" vlan_mode\u003dnative-untagged\n ovs-vsctl set port \"$( virsh dumpxml vm0 | xmlstarlet sel -t -v \u0027//interface[2]/target/@dev\u0027 )\" tag\u003d10\n dl_vlan\u003d10 does not match\n\n ovs-vsctl clear port \"$( virsh dumpxml vm0 | xmlstarlet sel -t -v \u0027//interface[2]/target/@dev\u0027 )\" tag\n ovs-vsctl set Port \"$( virsh dumpxml vm0 | xmlstarlet sel -t -v \u0027//interface[2]/target/@dev\u0027 )\" vlan_mode\u003dtrunk\n ovs-vsctl set port \"$( virsh dumpxml vm0 | xmlstarlet sel -t -v \u0027//interface[2]/target/@dev\u0027 )\" trunks\u003d10\n dl_vlan\u003d10 does not match","commit_id":"9c673cf48cc1dcfb8b15fb08cfd1c02a4ba1621e"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"b5e1a9f1f5588d818d2c992ce23d8f4d7bf86bbb","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"a563e800_da38e9db","in_reply_to":"aec69752_10aa1cb4","updated":"2022-01-27 13:24:04.000000000","message":"Or maybe setting vlan_mode\u003dtrunk, and trunks\u003d[4095] - packets should be dropped as have no 802.1Q header - which is considered as 0, and hence not allowed by \"trunks\" setting.","commit_id":"9c673cf48cc1dcfb8b15fb08cfd1c02a4ba1621e"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"b277b62b770bd25fd8ee1b3af7879a2ea8c9b9e1","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"fb5cc46b_9ab15535","in_reply_to":"be0d63ae_de2bb2ea","updated":"2022-02-01 14:07:46.000000000","message":"Agree, I will prepare a patch for the alternate approach meanwhile","commit_id":"9c673cf48cc1dcfb8b15fb08cfd1c02a4ba1621e"},{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"working for Ericsson, UTC+1 (+DST)"},"change_message_id":"1290325f35a1c7ccc8e74b4ef9169beafe08856f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"29b3a18a_ba4aa5bf","in_reply_to":"bee4eb5d_d5ffdf40","updated":"2022-02-01 12:10:13.000000000","message":"Sorry for the slow response. I did test what you suggested. It works, but with caveats.\n\nI plugged two libvirt vms into br0 and set their ports like this:\n\n for vm in vm0 vm1\n do\n   ovs-vsctl clear Port \"$( virsh dumpxml \"$vm\" | xmlstarlet sel -t -v \u0027//interface[2]/target/@dev\u0027 )\" tag\n   ovs-vsctl set Port \"$( virsh dumpxml \"$vm\" | xmlstarlet sel -t -v \u0027//interface[2]/target/@dev\u0027 )\" vlan_mode\u003dtrunk\n   ovs-vsctl set Port \"$( virsh dumpxml \"$vm\" | xmlstarlet sel -t -v \u0027//interface[2]/target/@dev\u0027 )\" trunks\u003d4095\n done\n\nIn this test environment I only had the single default normal action flow:\n\n # ovs-ofctl dump-flows br0\n  cookie\u003d0x0, duration\u003d345716.916s, table\u003d0, n_packets\u003d80851, n_bytes\u003d3441463, priority\u003d0 actions\u003dNORMAL\n\nIn this case any vlan that\u0027s not vlan 4095 gets dropped as you said. Which I could also confirm like this:\n\n # ovs-appctl ofproto/trace br0 in_port\u003d1,dl_vlan\u003d0\n Flow: in_port\u003d1,dl_vlan\u003d0,dl_vlan_pcp\u003d0,vlan_tci1\u003d0x0000,dl_src\u003d00:00:00:00:00:00,dl_dst\u003d00:00:00:00:00:00,dl_type\u003d0x0000\n·\n bridge(\"br0\")\n -------------\n  0. priority 0\n     NORMAL\n      \u003e\u003e\u003e\u003e dropping VLAN 0 packet received on port vnet1 not configured for trunking VLAN 0 \u003c\u003c\u003c\u003c\n      \u003e\u003e disallowed VLAN VID for this input port, dropping\n·\n Final flow: unchanged\n Megaflow: recirc_id\u003d0,eth,in_port\u003d1,dl_vlan\u003d0,dl_src\u003d00:00:00:00:00:00,dl_dst\u003d00:00:00:00:00:00,dl_type\u003d0x0000\n Datapath actions: drop\n\nBut we have caveats:\n\n1) If we implement the dead vlan this way, then all ports plugged into the dead vlan are able to communicate with each other on vlan 4095 (at least inside a compute host). vlan 4095 is a reserved vlan, so for example Linux does not allow a vlan subinterface with id 4095, but clearly if you can modify what\u0027s on the wire, you can use vlan 4095 as a valid vlan. ovs does not treat it in a special way. This already means that we must totally trust whatever is connected to a dead vlan port (all dhcp server and router implementations we allow). If they happen to have an independent vulnerability that may be combined with the fact, that they have a ready-made communication channel between each other crossing tenant separation.\n\n2) Keep in mind that the normal action flow(s) are always the last flows in the flow table. Anything connected to dead vlan ports may try subverting all flows before the normal action flow. For example flows handling traffic towards physical bridges. This can happen even outside vlan 4095.","commit_id":"9c673cf48cc1dcfb8b15fb08cfd1c02a4ba1621e"},{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"working for Ericsson, UTC+1 (+DST)"},"change_message_id":"bffa3b102488efe069b7de32c0a81e7f63a74652","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"fa6c3833_e4e5d63a","in_reply_to":"cb5ff399_aa30bab5","updated":"2022-02-01 12:37:26.000000000","message":"\u003e \u003e 1) If we implement the dead vlan this way, then all ports plugged into the dead vlan are able to communicate with each other on vlan 4095 (at least inside a compute host). vlan 4095 is a reserved vlan, so for example Linux does not allow a vlan subinterface with id 4095, but clearly if you can modify what\u0027s on the wire, you can use vlan 4095 as a valid vlan. ovs does not treat it in a special way. This already means that we must totally trust whatever is connected to a dead vlan port (all dhcp server and router implementations we allow). If they happen to have an independent vulnerability that may be combined with the fact, that they have a ready-made communication channel between each other crossing tenant separation.\n\u003e \u003e \n\u003e \n\u003e But we still have default drop rule in br-int for packets with vlan 4095 - that should not allow communication over that vlan, right? (Though I\u0027m not sure how dhcp/router ports could even try to do this)\n\nYou\u0027re right. We can drop that explicitly. We are actually already dropping it.\n\n\u003e \u003e 2) Keep in mind that the normal action flow(s) are always the last flows in the flow table. Anything connected to dead vlan ports may try subverting all flows before the normal action flow. For example flows handling traffic towards physical bridges. This can happen even outside vlan 4095.\n\u003e \n\u003e Not sure I got this. My vision is: new plugged dhcp/router port won\u0027t be able to send untagged packets or any tagged packets with vlan!\u003d4095 (because of configured trunks); on the other hand packets with vlan 4095 will be dropped by br-int.\n\u003e Am I missing something?\n\nThose ports can send packets in any vlan. Those packets will be processed first in the openflow flow table, according to whatever we have there (for example vlan remapping flows for physical bridges). Only when the flow processing gets to the last flow (which has actions\u003dNORMAL) will the normal switching pipeline actually drop those packets.","commit_id":"9c673cf48cc1dcfb8b15fb08cfd1c02a4ba1621e"},{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"working for Ericsson, UTC+1 (+DST)"},"change_message_id":"ad52c86cecb81aa0dc15073db4b13ecc053cc15d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"4de78ea2_a0cd6e06","in_reply_to":"cc14d607_cb18fd3e","updated":"2022-01-31 13:50:24.000000000","message":"Now I understand the idea. And it would be great if it worked. But I don\u0027t like what I see in ovs:\n\nI have flows like this:\n\n # ovs-ofctl dump-flows br0\n  cookie\u003d0x1, duration\u003d6233.867s, table\u003d0, n_packets\u003d0, n_bytes\u003d0, priority\u003d65535,dl_vlan\u003d10 actions\u003ddrop\n  cookie\u003d0x0, duration\u003d266008.628s, table\u003d0, n_packets\u003d4526, n_bytes\u003d213535, priority\u003d0 actions\u003dNORMAL\n\nAnd a port configured like this:\n\n # ovs-vsctl find Port name\u003d$( virsh dumpxml vm0 | xmlstarlet sel -t -v \u0027//interface[2]/target/@dev\u0027 )\n [snip]\n tag                 : []\n trunks              : [4095]\n vlan_mode           : trunk\n\nAnd I send (arp) traffic out from a vm plugged into that port by pinging a random address (where of course nobody responds).\n\nAnd then I see (the packet counter grows as the ping goes and it stops when the ping stops) that the normal flow matches these packets:\n\n # ovs-ofctl dump-flows br0\n  cookie\u003d0x1, duration\u003d6233.867s, table\u003d0, n_packets\u003d0, n_bytes\u003d0, priority\u003d65535,dl_vlan\u003d10 actions\u003ddrop\n  cookie\u003d0x0, duration\u003d266008.628s, table\u003d0, n_packets\u003d4526, n_bytes\u003d213535, priority\u003d0 actions\u003dNORMAL\n\nThat\u0027s why I cannot assume that a trunk only accepts the untagged traffic if the trunks list contains 0.\n\n\u003e We need to drop packets _from_ the new port in the first place, right?\n\nIf it applies to all ports, then yes it\u0027s enough to drop only in one direction. If we can drop both directions we have more room to make mistakes. :-)","commit_id":"9c673cf48cc1dcfb8b15fb08cfd1c02a4ba1621e"},{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"working for Ericsson, UTC+1 (+DST)"},"change_message_id":"1c432a2cf688d91922c6dd318800515ccdac2fbf","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"be0d63ae_de2bb2ea","in_reply_to":"cf599f0b_7a094d8c","updated":"2022-02-01 14:03:17.000000000","message":"Anyway, I believe on the neutron meeting starting now we should decide which approach to take. And until that approach is implemented we should either revert the first patch, or merge this patch and later revert the two together. But now the first patch without the second is breaking the gate AFAIU.","commit_id":"9c673cf48cc1dcfb8b15fb08cfd1c02a4ba1621e"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"44b022cf7cfdc41c4a809a05413c60675dbd2f03","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"cf599f0b_7a094d8c","in_reply_to":"ebdefdbb_be46e01d","updated":"2022-02-01 13:53:40.000000000","message":"\u003e \u003e Ah, got it. Still this is out of scope of the original bug, right? Also those ports normally don\u0027t do anything like that..\n\u003e \n\u003e Normally they don\u0027t.\n\nWell, in fact this is the same as with current fully configured DHCP ports: if they for some (bad) reason start sending tagged packets with some vlan, isn\u0027t it?","commit_id":"9c673cf48cc1dcfb8b15fb08cfd1c02a4ba1621e"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"8d33a80837b3b5b072ab38ce2102106a06cd5743","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"62046c50_2c9bef24","in_reply_to":"fa6c3833_e4e5d63a","updated":"2022-02-01 12:47:10.000000000","message":"\u003e \u003e \u003e 1) If we implement the dead vlan this way, then all ports plugged into the dead vlan are able to communicate with each other on vlan 4095 (at least inside a compute host). vlan 4095 is a reserved vlan, so for example Linux does not allow a vlan subinterface with id 4095, but clearly if you can modify what\u0027s on the wire, you can use vlan 4095 as a valid vlan. ovs does not treat it in a special way. This already means that we must totally trust whatever is connected to a dead vlan port (all dhcp server and router implementations we allow). If they happen to have an independent vulnerability that may be combined with the fact, that they have a ready-made communication channel between each other crossing tenant separation.\n\u003e \u003e \u003e \n\u003e \u003e \n\u003e \u003e But we still have default drop rule in br-int for packets with vlan 4095 - that should not allow communication over that vlan, right? (Though I\u0027m not sure how dhcp/router ports could even try to do this)\n\u003e \n\u003e You\u0027re right. We can drop that explicitly. We are actually already dropping it.\n\u003e \n\u003e \u003e \u003e 2) Keep in mind that the normal action flow(s) are always the last flows in the flow table. Anything connected to dead vlan ports may try subverting all flows before the normal action flow. For example flows handling traffic towards physical bridges. This can happen even outside vlan 4095.\n\u003e \u003e \n\u003e \u003e Not sure I got this. My vision is: new plugged dhcp/router port won\u0027t be able to send untagged packets or any tagged packets with vlan!\u003d4095 (because of configured trunks); on the other hand packets with vlan 4095 will be dropped by br-int.\n\u003e \u003e Am I missing something?\n\u003e \n\u003e Those ports can send packets in any vlan. Those packets will be processed first in the openflow flow table, according to whatever we have there (for example vlan remapping flows for physical bridges). Only when the flow processing gets to the last flow (which has actions\u003dNORMAL) will the normal switching pipeline actually drop those packets.\n\nAh, got it. Still this is out of scope of the original bug, right? Also those ports normally don\u0027t do anything like that..","commit_id":"9c673cf48cc1dcfb8b15fb08cfd1c02a4ba1621e"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"dcc51c789f0c70b01dac781286335a2e307539ab","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"d0fa27a6_a9c678b1","updated":"2022-01-27 14:53:23.000000000","message":"-1 to discuss/try my ideas. Actually I tried the with \"trunk\" and \"native-tagged\" vlan_modes - and it seems working and drops traffic from the port","commit_id":"ae1a4989063c6c6459d8b751439eb20d58c6b03a"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"310035bd52a38d23675893cdffa1ad98daa85930","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"e77e36ee_5b40e64e","updated":"2022-01-27 17:06:33.000000000","message":"-1 just for visibility","commit_id":"43a8185cb8353aac904953f0ee007f28e0bd5cc2"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"559afc2e78560ec3899bdbae12df99db8388aca5","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"71d92687_52a5d195","updated":"2022-02-01 16:09:33.000000000","message":"Alternative approach: https://review.opendev.org/c/openstack/neutron/+/827315","commit_id":"43a8185cb8353aac904953f0ee007f28e0bd5cc2"},{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"working for Ericsson, UTC+1 (+DST)"},"change_message_id":"2e8bd295dc6dad658c7802521c661ebec3718d1b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"1decff1f_18e11bfe","updated":"2022-01-28 09:14:16.000000000","message":"I will check all ideas we gathered here to see if we have a less problematic approach to solve the original bug.\n\nUntil then I just wanted to add some notes about how to reproduce the problems here (all in a single host devstack). Unfortunately all of these rely on adding sleep()s, that\u0027s why these changes don\u0027t have proper tests.\n\n1) To reproduce the original security bug, before the previous change was merged:\n\nAdd sleep() to make the original bug practically deterministic:\nhttps://opendev.org/openstack/neutron/src/commit/da9fd503cef53ae3f9701d57bf1f1bb388129107/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py#L1232\n\nCreate a bunch of subnets with the same range, therefore neutron picks the same dhcp port addresses, for which ipv6 duplicate address detection catches when the dead vlan leaks:\n for i in {1..5}\n do\n    ( openstack network create xnet$i ; openstack subnet create xsubnet$i-v6 --ip-version 6 --network xnet$i --subnet-range 2001:db8::/32 ) \u0026\n done\n\nWhen the bug is present, usually all but one dhcp port addresses go into the dadfailed state.\nWhen the bug is fixed there\u0027s no dadfailed address.\n for net in $( openstack network list -f value -c Name -c ID | awk \u0027/ xnet/ { print $1 }\u0027 ) ; do sudo ip netns exec qdhcp-$net ip a ; done | egrep \u0027inet6 2001:db8::\u0027\n\n2) To exercise the race condition addressed in this change:\n\nAdd sleep() to force the delete_flows, add_flow order:\nhttps://opendev.org/openstack/neutron/src/commit/7aae31c9f9ed938760ca0be3c461826b598c7004/neutron/agent/common/ovs_lib.py#L399\n\nAdd sleep() to force the add_flow, delete_flows order:\nhttps://opendev.org/openstack/neutron/src/commit/7aae31c9f9ed938760ca0be3c461826b598c7004/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py#L1239\n\nWith the wrong order we leave a port dead, which can be detected by many tests, for example this:\n tempest.scenario.test_network_basic_ops.TestNetworkBasicOps.test_router_rescheduling\n\nAnd after each of these failures we leave behind one of these flows:\n sudo ovs-ofctl dump-flows br-int | egrep -c priority\u003d65534\n","commit_id":"43a8185cb8353aac904953f0ee007f28e0bd5cc2"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"6185b3ad4b7caa3106b5d28669e08f8df484b238","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"4c964bc8_8548a4a6","updated":"2022-01-28 07:04:34.000000000","message":"Please check my comments on PS2","commit_id":"43a8185cb8353aac904953f0ee007f28e0bd5cc2"}],"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":"7cb2a47356801072b2bee29553fd1f54fcb027c3","unresolved":true,"context_lines":[{"line_number":1234,"context_line":"                # something else, also delete the previously dead port\u0027s"},{"line_number":1235,"context_line":"                # push DEAD_VLAN_TAG flow which we installed from"},{"line_number":1236,"context_line":"                # ovs_lib.OVSBridge.replace_port()."},{"line_number":1237,"context_line":"                if (cur_tag \u003d\u003d constants.DEAD_VLAN_TAG and port.ofport !\u003d -1):"},{"line_number":1238,"context_line":"                    def count_dead_vlan_flows(ofport):"},{"line_number":1239,"context_line":"                        flows \u003d self.int_br.dump_flows_for("},{"line_number":1240,"context_line":"                            table\u003dconstants.LOCAL_SWITCHING,"}],"source_content_type":"text/x-python","patch_set":1,"id":"6a6ea11c_64459592","line":1237,"range":{"start_line":1237,"start_character":74,"end_line":1237,"end_character":76},"updated":"2022-01-27 09:09:13.000000000","message":"please use constant here too","commit_id":"9c673cf48cc1dcfb8b15fb08cfd1c02a4ba1621e"},{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"working for Ericsson, UTC+1 (+DST)"},"change_message_id":"e17f1b963d7ff23b76691e0fef298a187c7049b1","unresolved":false,"context_lines":[{"line_number":1234,"context_line":"                # something else, also delete the previously dead port\u0027s"},{"line_number":1235,"context_line":"                # push DEAD_VLAN_TAG flow which we installed from"},{"line_number":1236,"context_line":"                # ovs_lib.OVSBridge.replace_port()."},{"line_number":1237,"context_line":"                if (cur_tag \u003d\u003d constants.DEAD_VLAN_TAG and port.ofport !\u003d -1):"},{"line_number":1238,"context_line":"                    def count_dead_vlan_flows(ofport):"},{"line_number":1239,"context_line":"                        flows \u003d self.int_br.dump_flows_for("},{"line_number":1240,"context_line":"                            table\u003dconstants.LOCAL_SWITCHING,"}],"source_content_type":"text/x-python","patch_set":1,"id":"ee59d411_604ec9a5","line":1237,"range":{"start_line":1237,"start_character":74,"end_line":1237,"end_character":76},"in_reply_to":"6a6ea11c_64459592","updated":"2022-01-27 10:25:05.000000000","message":"Done","commit_id":"9c673cf48cc1dcfb8b15fb08cfd1c02a4ba1621e"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"029112c389a110ecaaecd05ba0b7df0fdeefafdf","unresolved":true,"context_lines":[{"line_number":1235,"context_line":"                # push DEAD_VLAN_TAG flow which we installed from"},{"line_number":1236,"context_line":"                # ovs_lib.OVSBridge.replace_port()."},{"line_number":1237,"context_line":"                if (cur_tag \u003d\u003d constants.DEAD_VLAN_TAG and port.ofport !\u003d -1):"},{"line_number":1238,"context_line":"                    def count_dead_vlan_flows(ofport):"},{"line_number":1239,"context_line":"                        flows \u003d self.int_br.dump_flows_for("},{"line_number":1240,"context_line":"                            table\u003dconstants.LOCAL_SWITCHING,"},{"line_number":1241,"context_line":"                            in_port\u003dofport,"}],"source_content_type":"text/x-python","patch_set":1,"id":"39c38543_a747faef","line":1238,"updated":"2022-01-27 08:52:50.000000000","message":"nitty nit: maybe for better readability You can add empty line before and after definition of that function?","commit_id":"9c673cf48cc1dcfb8b15fb08cfd1c02a4ba1621e"},{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"working for Ericsson, UTC+1 (+DST)"},"change_message_id":"e17f1b963d7ff23b76691e0fef298a187c7049b1","unresolved":false,"context_lines":[{"line_number":1235,"context_line":"                # push DEAD_VLAN_TAG flow which we installed from"},{"line_number":1236,"context_line":"                # ovs_lib.OVSBridge.replace_port()."},{"line_number":1237,"context_line":"                if (cur_tag \u003d\u003d constants.DEAD_VLAN_TAG and port.ofport !\u003d -1):"},{"line_number":1238,"context_line":"                    def count_dead_vlan_flows(ofport):"},{"line_number":1239,"context_line":"                        flows \u003d self.int_br.dump_flows_for("},{"line_number":1240,"context_line":"                            table\u003dconstants.LOCAL_SWITCHING,"},{"line_number":1241,"context_line":"                            in_port\u003dofport,"}],"source_content_type":"text/x-python","patch_set":1,"id":"61afc91a_79ba1306","line":1238,"in_reply_to":"39c38543_a747faef","updated":"2022-01-27 10:25:05.000000000","message":"Done","commit_id":"9c673cf48cc1dcfb8b15fb08cfd1c02a4ba1621e"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"029112c389a110ecaaecd05ba0b7df0fdeefafdf","unresolved":true,"context_lines":[{"line_number":1249,"context_line":"                        if count_dead_vlan_flows(port.ofport) \u003e\u003d 1:"},{"line_number":1250,"context_line":"                            saw_dead_vlan_flows \u003d True"},{"line_number":1251,"context_line":"                            break"},{"line_number":1252,"context_line":"                        time.sleep(1)"},{"line_number":1253,"context_line":"                    if saw_dead_vlan_flows:"},{"line_number":1254,"context_line":"                        self.int_br.delete_flows("},{"line_number":1255,"context_line":"                            cookie\u003dovs_lib.COOKIE_ANY,"}],"source_content_type":"text/x-python","patch_set":1,"id":"cea00f8f_9c5c7007","line":1252,"updated":"2022-01-27 08:52:50.000000000","message":"maybe instead of this loop You can use \"wait_until_true\" helper function here?","commit_id":"9c673cf48cc1dcfb8b15fb08cfd1c02a4ba1621e"},{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"working for Ericsson, UTC+1 (+DST)"},"change_message_id":"e17f1b963d7ff23b76691e0fef298a187c7049b1","unresolved":false,"context_lines":[{"line_number":1249,"context_line":"                        if count_dead_vlan_flows(port.ofport) \u003e\u003d 1:"},{"line_number":1250,"context_line":"                            saw_dead_vlan_flows \u003d True"},{"line_number":1251,"context_line":"                            break"},{"line_number":1252,"context_line":"                        time.sleep(1)"},{"line_number":1253,"context_line":"                    if saw_dead_vlan_flows:"},{"line_number":1254,"context_line":"                        self.int_br.delete_flows("},{"line_number":1255,"context_line":"                            cookie\u003dovs_lib.COOKIE_ANY,"}],"source_content_type":"text/x-python","patch_set":1,"id":"cdb85c52_58cb8e42","line":1252,"in_reply_to":"cea00f8f_9c5c7007","updated":"2022-01-27 10:25:05.000000000","message":"Done","commit_id":"9c673cf48cc1dcfb8b15fb08cfd1c02a4ba1621e"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"310035bd52a38d23675893cdffa1ad98daa85930","unresolved":true,"context_lines":[{"line_number":1257,"context_line":"                            table\u003dconstants.LOCAL_SWITCHING,"},{"line_number":1258,"context_line":"                            priority\u003dconstants.OPENFLOW_MAX_PRIORITY - 1,"},{"line_number":1259,"context_line":"                            in_port\u003dport.ofport,"},{"line_number":1260,"context_line":"                            strict\u003dTrue)"},{"line_number":1261,"context_line":"                    except n_utils.WaitTimeout:"},{"line_number":1262,"context_line":"                        LOG.warning("},{"line_number":1263,"context_line":"                            \u0027Expected to find and delete dead vlan flow, \u0027"}],"source_content_type":"text/x-python","patch_set":3,"id":"716f1b8a_23658b38","line":1260,"range":{"start_line":1260,"start_character":28,"end_line":1260,"end_character":40},"updated":"2022-01-27 17:06:33.000000000","message":"The port deletion is executed first because \"strict\u003dTrue\". Check \"do_action_flows\" method. The \"strict\" and \"non-strict\" flows can\u0027t be executed in the same bundle. \"strict\" flow commands (deletion), are executed first.\n\nCheck [1] commit message and bug. This is something similar.\n\nIn this case, same as in [1], I think you can avoid \"strict\" flag removing the \"priority\" field. \"priority\" cannot be executed without \"strict\". But in this case doesn\u0027t matter: \"in_port\u003dport.ofport\" must be unique, that means this is enough to find the flow we need to remove.\n\nIf you have an environment ready to reproduce this issue, I would first remove \"strict\" and \"priority\" fields from this command.\n\n[1]https://review.opendev.org/c/openstack/neutron/+/819900","commit_id":"43a8185cb8353aac904953f0ee007f28e0bd5cc2"},{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"working for Ericsson, UTC+1 (+DST)"},"change_message_id":"2e8bd295dc6dad658c7802521c661ebec3718d1b","unresolved":true,"context_lines":[{"line_number":1257,"context_line":"                            table\u003dconstants.LOCAL_SWITCHING,"},{"line_number":1258,"context_line":"                            priority\u003dconstants.OPENFLOW_MAX_PRIORITY - 1,"},{"line_number":1259,"context_line":"                            in_port\u003dport.ofport,"},{"line_number":1260,"context_line":"                            strict\u003dTrue)"},{"line_number":1261,"context_line":"                    except n_utils.WaitTimeout:"},{"line_number":1262,"context_line":"                        LOG.warning("},{"line_number":1263,"context_line":"                            \u0027Expected to find and delete dead vlan flow, \u0027"}],"source_content_type":"text/x-python","patch_set":3,"id":"fcad352c_2593940c","line":1260,"range":{"start_line":1260,"start_character":28,"end_line":1260,"end_character":40},"in_reply_to":"716f1b8a_23658b38","updated":"2022-01-28 09:14:16.000000000","message":"It\u0027s an interesting point to not filter for priority and then we don\u0027t need strict\u003dTrue. However please keep in mind that add_flow and delete_flows are executed from different processes (at least as long as we don\u0027t change the previous patch\u0027s approach). I see how using strict may change the timing of flow operations inside a process. But I don\u0027t see how it would change the transaction ordering between two processes. Am I missing something here?","commit_id":"43a8185cb8353aac904953f0ee007f28e0bd5cc2"},{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"working for Ericsson, UTC+1 (+DST)"},"change_message_id":"f9f7a87783f3c103de34021193e25b36599362a3","unresolved":false,"context_lines":[{"line_number":1257,"context_line":"                            table\u003dconstants.LOCAL_SWITCHING,"},{"line_number":1258,"context_line":"                            priority\u003dconstants.OPENFLOW_MAX_PRIORITY - 1,"},{"line_number":1259,"context_line":"                            in_port\u003dport.ofport,"},{"line_number":1260,"context_line":"                            strict\u003dTrue)"},{"line_number":1261,"context_line":"                    except n_utils.WaitTimeout:"},{"line_number":1262,"context_line":"                        LOG.warning("},{"line_number":1263,"context_line":"                            \u0027Expected to find and delete dead vlan flow, \u0027"}],"source_content_type":"text/x-python","patch_set":3,"id":"2eab0448_f4dfd4b4","line":1260,"range":{"start_line":1260,"start_character":28,"end_line":1260,"end_character":40},"in_reply_to":"fcad352c_2593940c","updated":"2022-01-31 12:17:14.000000000","message":"We discussed this in the neutron channel and agreed strict\u003dFalse won\u0027t eliminate the race between the two agents.","commit_id":"43a8185cb8353aac904953f0ee007f28e0bd5cc2"}]}