)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"bf5259a59fbd3335f34e62832c304e1fe5429b2f","unresolved":true,"context_lines":[{"line_number":15,"context_line":"While working on this I realized multiple related problems:"},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"Nobody puts the trunk tpi and spi ports in the dead vlan either. For"},{"line_number":18,"context_line":"this see related neutron change I4f80d94bc1f346fd0a7ad0b9d684792f1451cf24"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"In neutron we also changed how the dead vlan is handled. Instead of"},{"line_number":21,"context_line":"simply setting tag\u003d4095 we set (beyond having a drop rule for tag 4095):"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"28b1bf6e_e48dd97c","line":18,"updated":"2024-07-01 12:06:23.000000000","message":"that is not required because those are create by neutron.\n\nos-vif puts port in the dead vlan because we do not know what vlan will be used on the integration brindge.\n\nsince the patch port are created by neutron it does know what valn to use so there is no reason to put them in the dead vlan.","commit_id":"09447517c4a64ee07eb2a4089dd6a04fca569368"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"e1545a3c1764ff376b7449bcbcf249f275f12f40","unresolved":true,"context_lines":[{"line_number":15,"context_line":"While working on this I realized multiple related problems:"},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"Nobody puts the trunk tpi and spi ports in the dead vlan either. For"},{"line_number":18,"context_line":"this see related neutron change I4f80d94bc1f346fd0a7ad0b9d684792f1451cf24"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"In neutron we also changed how the dead vlan is handled. Instead of"},{"line_number":21,"context_line":"simply setting tag\u003d4095 we set (beyond having a drop rule for tag 4095):"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"8ed2d17e_bebfdfc6","line":18,"in_reply_to":"04acf6de_a6b28c68","updated":"2024-07-03 11:54:54.000000000","message":"i would consider that a security bug in neutron.\n\nwe very intentially use a transaction when creating the prot in the ovs-db in os-vif to ensure that the port is atomicaly created with all configuration for tenatn isolation\n\nthis is supproted with both the ovs-vsctl command line interface and the python bidnigns ot this was never a limitation fo how neutron is configuring ovs via either client.\n\n\novs has the concept of a bridge contoelr fail mode and the br-int is inteed to be deploy in secure mode http://www.openvswitch.org/support/dist-docs/ovs-vsctl.8.txt\n\n```\nController Failure Settings\n\n       When  a  controller  is  configured, it is, ordinarily, responsible for\n       setting up all flows on the switch.  Thus, if  the  connection  to  the\n       controller  fails,  no  new  network connections can be set up.  If the\n       connection to the controller stays down long  enough,  no  packets  can\n       pass through the switch at all.\n\n       If  the  value  is  standalone, or if neither of these settings is set,\n       ovs-vswitchd will take over responsibility for setting up flows when no\n       message has been received from the controller for three times the inac‐\n       tivity probe interval.  In this mode, ovs-vswitchd causes the  datapath\n       to  act  like  an ordinary MAC-learning switch.  ovs-vswitchd will con‐\n       tinue to retry connecting to the controller in the background and, when\n       the connection succeeds, it discontinues its standalone behavior.\n\n       If this option is set to secure, ovs-vswitchd will not set up flows  on\n       its own when the controller connection fails.\n\n       get-fail-mode bridge\n              Prints the configured failure mode.\n\n       del-fail-mode bridge\n              Deletes the configured failure mode.\n\n       set-fail-mode bridge standalone|secure\n              Sets the configured failure mode.\n```\n\nwhen set to secure it will install a default drop rule.\n\nfurther more per http://magrawal.myweb.usf.edu/dcom/Ch3_802.1Q-2005.pdf#G13.67436 vlan 4095 is reserved\n\n```\nReserved for implementation use. This VID value shall not be configured as a\nPVID or a member of a VID Set, or transmitted in a tag header. This VID value\nmay be used to indicate a wildcard match for the VID in management operations\nor Filtering Database entries\n```\n\nin other words ovs is not allowed to transmit a frame with vlan tag 4095 (0xFFF)","commit_id":"09447517c4a64ee07eb2a4089dd6a04fca569368"},{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"working for Ericsson, UTC+1 (+DST)"},"change_message_id":"684271bfe4f9988b623d1b896c51e247edff3982","unresolved":true,"context_lines":[{"line_number":15,"context_line":"While working on this I realized multiple related problems:"},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"Nobody puts the trunk tpi and spi ports in the dead vlan either. For"},{"line_number":18,"context_line":"this see related neutron change I4f80d94bc1f346fd0a7ad0b9d684792f1451cf24"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"In neutron we also changed how the dead vlan is handled. Instead of"},{"line_number":21,"context_line":"simply setting tag\u003d4095 we set (beyond having a drop rule for tag 4095):"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"04acf6de_a6b28c68","line":18,"in_reply_to":"28b1bf6e_e48dd97c","updated":"2024-07-02 11:49:34.000000000","message":"I abandoned the relevant neutron patch. However I\u0027m still not fully convinced it\u0027s not needed. While neutron theoretically could set the tag on the tpi/spi ports at the time of creation, I believe that\u0027s not what\u0027s happening and we have a time window between creation and being put in the proper vlan. As you can see in this output while booting a vm with a trunk (though ovsdb-client does not show transaction boundaries):\n\n$ sudo ovsdb-client monitor Port name tag trunks vlan_mode\n[snip]\nrow                                  action name           tag trunks vlan_mode\n------------------------------------ ------ -------------- --- ------ ---------\n9b779622-afc7-4f55-ab46-565f340907a4 insert tbr-a375bc09-7 []  []     []\n\nrow                                  action name           tag trunks vlan_mode\n------------------------------------ ------ -------------- --- ------ ---------\n1677a4c1-9a91-4618-98b0-e5095ceb87ef insert tapcf2856f9-65 []  []     []\n\nrow                                  action name            tag trunks vlan_mode\n------------------------------------ ------ --------------- --- ------ ---------\ncc26ac2c-3b6c-44ee-8d9d-46d8b8ddae01 insert tpi-cf2856f9-65 []  []     []\n96a28f1d-0b79-47c3-8c88-ccef4d5502e2 insert tpt-cf2856f9-65 0   []     access\n\nrow                                  action name            tag trunks vlan_mode\n------------------------------------ ------ --------------- --- ------ ---------\na68aa8f2-62dd-4d62-980d-eb2df88c82bf insert spi-7914d497-b6 []  []     []\n8dc56e92-7403-4b99-8593-54f701c9c8a4 insert spt-7914d497-b6 101 []     access\n\nrow                                  action name            tag trunks vlan_mode\n------------------------------------ ------ --------------- --- ------ ---------\na68aa8f2-62dd-4d62-980d-eb2df88c82bf old                    []\n                                     new    spi-7914d497-b6 2   []     []\n\nrow                                  action name            tag trunks vlan_mode\n------------------------------------ ------ --------------- --- ------ ---------\ncc26ac2c-3b6c-44ee-8d9d-46d8b8ddae01 old                    []\n                                     new    tpi-cf2856f9-65 1   []     []","commit_id":"09447517c4a64ee07eb2a4089dd6a04fca569368"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"bf5259a59fbd3335f34e62832c304e1fe5429b2f","unresolved":true,"context_lines":[{"line_number":19,"context_line":""},{"line_number":20,"context_line":"In neutron we also changed how the dead vlan is handled. Instead of"},{"line_number":21,"context_line":"simply setting tag\u003d4095 we set (beyond having a drop rule for tag 4095):"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"tag : 4095"},{"line_number":24,"context_line":"trunks : [4095]"},{"line_number":25,"context_line":"vlan_mode : trunk"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"3288d292_4fce6f2b","line":22,"updated":"2024-07-01 12:06:23.000000000","message":"this should nto be required since all RFC compliten siwthc are required to drop 4095 anyway and we run the br-int in secure mode which adds a default drop rule.","commit_id":"09447517c4a64ee07eb2a4089dd6a04fca569368"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"e1545a3c1764ff376b7449bcbcf249f275f12f40","unresolved":true,"context_lines":[{"line_number":19,"context_line":""},{"line_number":20,"context_line":"In neutron we also changed how the dead vlan is handled. Instead of"},{"line_number":21,"context_line":"simply setting tag\u003d4095 we set (beyond having a drop rule for tag 4095):"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"tag : 4095"},{"line_number":24,"context_line":"trunks : [4095]"},{"line_number":25,"context_line":"vlan_mode : trunk"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"9e21ddce_f2f6cc6e","line":22,"in_reply_to":"0c7bcafe_ad07a4b5","updated":"2024-07-03 11:54:54.000000000","message":"right having a high periortiy rule in the intiall ingress table that drops all traffic on vlan 4095 is fine\n\nin secure mode the only flow that will be on a bridge is the drop flow so all traffic will be droped unless neutron installs a flow.\n\nneutron technmically shoudl nto need to drop it but it does not hurt to do that explcitly.\n\nwhen neutron installs normal action flows it does that per local vlan and never installs one on vlan 4095.","commit_id":"09447517c4a64ee07eb2a4089dd6a04fca569368"},{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"working for Ericsson, UTC+1 (+DST)"},"change_message_id":"684271bfe4f9988b623d1b896c51e247edff3982","unresolved":true,"context_lines":[{"line_number":19,"context_line":""},{"line_number":20,"context_line":"In neutron we also changed how the dead vlan is handled. Instead of"},{"line_number":21,"context_line":"simply setting tag\u003d4095 we set (beyond having a drop rule for tag 4095):"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"tag : 4095"},{"line_number":24,"context_line":"trunks : [4095]"},{"line_number":25,"context_line":"vlan_mode : trunk"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"0c7bcafe_ad07a4b5","line":22,"in_reply_to":"3288d292_4fce6f2b","updated":"2024-07-02 11:49:34.000000000","message":"Maybe unnecessarily, but we do have a drop rule for vlan 4095:\n\n$ sudo ovs-ofctl dump-flows br-int | egrep 4095\n cookie\u003d0x79e203f4ef182b1c, duration\u003d80621.513s, table\u003d0, n_packets\u003d0, n_bytes\u003d0, idle_age\u003d65534, hard_age\u003d65534, priority\u003d65535,dl_vlan\u003d4095 actions\u003ddrop\n\nOne reason I guess is that ovs *does not* drop vlan 4095 - no matter what the rfc says.","commit_id":"09447517c4a64ee07eb2a4089dd6a04fca569368"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"bf5259a59fbd3335f34e62832c304e1fe5429b2f","unresolved":true,"context_lines":[{"line_number":26,"context_line":""},{"line_number":27,"context_line":"We do this not with the intention of making the port a trunk, but with"},{"line_number":28,"context_line":"the intention of dropping all traffic arriving on the port. For details"},{"line_number":29,"context_line":"please see the following patches:"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"neutron $ git log --oneline --no-merges --grep\u003d\u0027dead vlan\u0027 -2"},{"line_number":32,"context_line":"0ddca28454 Make sure \"dead vlan\" ports cannot transmit packets"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"bf86a237_4a9a3305","line":29,"updated":"2024-07-01 12:06:23.000000000","message":"this seams incorrect to me and i would not be infavor of backporting this in this change.\n\nif we want to make this non backporatble change in the neutron behavior then we need to split that into a sepreate patch.\notherwise this will be a master only change in behavior that we cant abckport to any release.\n\ni am not convidce this impoves the security so i would prefer to not modify neutron or include setting vlan_mode in os-vif","commit_id":"09447517c4a64ee07eb2a4089dd6a04fca569368"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"e1545a3c1764ff376b7449bcbcf249f275f12f40","unresolved":true,"context_lines":[{"line_number":26,"context_line":""},{"line_number":27,"context_line":"We do this not with the intention of making the port a trunk, but with"},{"line_number":28,"context_line":"the intention of dropping all traffic arriving on the port. For details"},{"line_number":29,"context_line":"please see the following patches:"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"neutron $ git log --oneline --no-merges --grep\u003d\u0027dead vlan\u0027 -2"},{"line_number":32,"context_line":"0ddca28454 Make sure \"dead vlan\" ports cannot transmit packets"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"9ab9d814_f7138a3c","line":29,"in_reply_to":"1a2cf852_99656225","updated":"2024-07-03 11:54:54.000000000","message":"the change to using vlan_mode\u003dtrunk is the problematic change as it would require the neutron change to be backported before we can backport the os-vif change.","commit_id":"09447517c4a64ee07eb2a4089dd6a04fca569368"},{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"working for Ericsson, UTC+1 (+DST)"},"change_message_id":"684271bfe4f9988b623d1b896c51e247edff3982","unresolved":true,"context_lines":[{"line_number":26,"context_line":""},{"line_number":27,"context_line":"We do this not with the intention of making the port a trunk, but with"},{"line_number":28,"context_line":"the intention of dropping all traffic arriving on the port. For details"},{"line_number":29,"context_line":"please see the following patches:"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"neutron $ git log --oneline --no-merges --grep\u003d\u0027dead vlan\u0027 -2"},{"line_number":32,"context_line":"0ddca28454 Make sure \"dead vlan\" ports cannot transmit packets"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"1a2cf852_99656225","line":29,"in_reply_to":"bf86a237_4a9a3305","updated":"2024-07-02 11:49:34.000000000","message":"The neutron patches changing how the dead vlan works (also referred from the commit message) are already backported to victoria, therefore I believe this patch could be backported also. Please elaborate if you see other reasons blocking a possible backport.","commit_id":"09447517c4a64ee07eb2a4089dd6a04fca569368"},{"author":{"_account_id":8313,"name":"Lajos Katona","display_name":"lajoskatona","email":"katonalala@gmail.com","username":"elajkat","status":"Ericsson Software Technology"},"change_message_id":"4aa0e5912e1adecc277ff1d4334a6baf7ca4c965","unresolved":true,"context_lines":[{"line_number":29,"context_line":"please see the following patches:"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"neutron $ git log --oneline --no-merges --grep\u003d\u0027dead vlan\u0027 -2"},{"line_number":32,"context_line":"0ddca28454 Make sure \"dead vlan\" ports cannot transmit packets"},{"line_number":33,"context_line":"7aae31c9f9 Make the dead vlan actually dead"},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"Therefore this patch also starts handling ports in the dead vlan as"},{"line_number":36,"context_line":"neutron does."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"821cce4e_881c2374","line":33,"range":{"start_line":32,"start_character":0,"end_line":33,"end_character":43},"updated":"2024-07-02 09:25:38.000000000","message":"These are the patch series:\nhttps://review.opendev.org/q/Ib6b70114efb140cf1393b57ebc350fea4b0a2443\nhttps://review.opendev.org/q/I0391dd24224f8656a09ddb002e7dae8783ba37a4","commit_id":"09447517c4a64ee07eb2a4089dd6a04fca569368"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"ff9bb997ecf12002034572d2f1b86de1112ebc50","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"25e15a56_1c62ba45","updated":"2024-06-28 13:09:07.000000000","message":"i need ot refelct on this but im very reluctant to change this lightly","commit_id":"f65641526433866985f2047b37c672662f7b1bea"},{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"working for Ericsson, UTC+1 (+DST)"},"change_message_id":"684271bfe4f9988b623d1b896c51e247edff3982","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"4d28a16f_24d1bb30","updated":"2024-07-02 11:49:34.000000000","message":"Thanks for the feedback! It feels like we may not have the same context, please also see the older patches affecting neutron\u0027s dead vlan handling.","commit_id":"09447517c4a64ee07eb2a4089dd6a04fca569368"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"08dc8927e6a376bd35114eb2d0b63d5604371d12","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"17ba0ac4_cf017983","updated":"2024-07-03 06:55:48.000000000","message":"-1 for visibility.\n\nI know we have implemented this \"trick\" before in the OVS agent (the two patches reported in the commit message). But I have one question: this os-vif path is also used to create a port with ML2/OVN. How it works with this mech driver?","commit_id":"9a88608e89745dba8a4857eca00c8c711c3247aa"},{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"working for Ericsson, UTC+1 (+DST)"},"change_message_id":"5273efb05975f3dc78010965ab4188befe99ffa6","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"d3a14b00_98e6d820","updated":"2024-07-03 14:38:08.000000000","message":"I created the below do-not-merge change to test ps3 of this change by the neutron-tempest-plugin-openvswitch and neutron-tempest-plugin-ovn jobs:\n\nhttps://review.opendev.org/c/openstack/neutron-tempest-plugin/+/923389","commit_id":"9a88608e89745dba8a4857eca00c8c711c3247aa"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"dd8f9c05bd8ce96c89eff91cefc373d36e596ca9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"f3b59785_976fec6c","in_reply_to":"265d66d7_f59444ae","updated":"2024-07-05 10:16:09.000000000","message":"i woudl strongly hope the neutron-tempest-plugin-ovn one would since the isolate_vif config option is not support or set to true for ovn.\n\nits only used with ml2/ovs :)","commit_id":"9a88608e89745dba8a4857eca00c8c711c3247aa"},{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"working for Ericsson, UTC+1 (+DST)"},"change_message_id":"4de3c52d9df76631691749e6e2403000be2f7b6f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"265d66d7_f59444ae","in_reply_to":"ab790592_af352797","updated":"2024-07-05 07:42:01.000000000","message":"The neutron-tempest-plugin-openvswitch and neutron-tempest-plugin-ovn jobs passed for ps4 of this change:\n\nhttps://review.opendev.org/c/openstack/neutron-tempest-plugin/+/923389/3#message-12603b1115aad442c1a93e6f434d069bdb4318c9","commit_id":"9a88608e89745dba8a4857eca00c8c711c3247aa"},{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"working for Ericsson, UTC+1 (+DST)"},"change_message_id":"f97b6df08f7bf1f187995608530561d926c0326d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"ab790592_af352797","in_reply_to":"d3a14b00_98e6d820","updated":"2024-07-04 15:04:54.000000000","message":"The neutron-tempest-plugin-openvswitch and neutron-tempest-plugin-ovn jobs passed for ps3 of this change:\n\nhttps://review.opendev.org/c/openstack/neutron-tempest-plugin/+/923389/2#message-7920ca19f1ec1945c84397d700e6a88ee5baa993","commit_id":"9a88608e89745dba8a4857eca00c8c711c3247aa"},{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"working for Ericsson, UTC+1 (+DST)"},"change_message_id":"29c1aa72eec500f1f4bde1229fe6574f85c553e7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"253aedfd_8cfe3f9e","in_reply_to":"f3b59785_976fec6c","updated":"2024-07-05 10:51:32.000000000","message":"Feel free to click at the link and see that the ovn job was run with OS_VIF_OVS_ISOLATE_VIF: False\n\nhttps://review.opendev.org/c/openstack/neutron-tempest-plugin/+/923389/3/zuul.d/master_jobs.yaml","commit_id":"9a88608e89745dba8a4857eca00c8c711c3247aa"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"89b9ff942de155d66c3a29a339ae240ddf0e4e76","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"fb216573_59aa1c81","updated":"2024-11-28 13:45:54.000000000","message":"I\u0027ll prefer Brian and Lajos to take a last review. Next week I\u0027ll merge it if they agree with this code.","commit_id":"545989b411ab3dad2dd5aa5cc55fe83943180d70"},{"author":{"_account_id":8313,"name":"Lajos Katona","display_name":"lajoskatona","email":"katonalala@gmail.com","username":"elajkat","status":"Ericsson Software Technology"},"change_message_id":"beb41bece0d2974732722e5b515f44b46eaee07d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"5f3946e1_0cadae89","updated":"2024-12-13 12:02:49.000000000","message":"Thanks for the patch and the really useful and thoguht provoking discussion:-)","commit_id":"545989b411ab3dad2dd5aa5cc55fe83943180d70"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"b3c63c54bda75d6073ca982f2da7cc3e87cb2146","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":4,"id":"74e861b3_bf145a5f","updated":"2024-08-27 14:38:30.000000000","message":"by the way when we were first fixing the cve we chose 4095 becuase ovs is requried to never transmit that outside the switch based on teh vlan spec\n\nthe alterive we considered was ovs-ofctl mod-port switch port down \u003cport\u003e\n\n       mod-port switch port action\n              Modify  characteristics  of port port in switch.  port may be an\n              OpenFlow port number or name (unless --no-names is specified) or\n              the keyword LOCAL (the preferred way to refer  to  the  OpenFlow\n              local port).  The action may be any one of the following:\n              up\n              down   Enable  or  disable the interface.  This is equivalent to\n                     ip link set up or ip link set down on a Unix system.\n                     \nwe chose not to do that becasue os-vif did not have a depency on ovs-ofctl at the time and it would still technially have a small windoer between the port add and the down call.\n\nreading https://mail.openvswitch.org/pipermail/ovs-discuss/2021-December/051647.html im pretty sure that either the beahivor described there depend on if you are runging ovs in secure mode or the change the behvior after we fixed the orgianl cve as that does not map with the behvior we obseved at the time we added \n\n\ngiven neturon has backproted supprot for ovs port of type turnk to zed\n\nim relucatntly inclidned to belive we will have to wap to using that here also even if that is a backward incompatable change.\n\n\nwithout this change it appears that https://bugs.launchpad.net/neutron/+bug/1734320\nhas been regressed so this is effectivly a public unpatched cve again...","commit_id":"545989b411ab3dad2dd5aa5cc55fe83943180d70"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"89da11db5e9941631766f822732c04a556a2ea1d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"4cbea811_25c34e83","updated":"2024-08-27 14:56:02.000000000","message":"this is still missing functional test coverage by the way.","commit_id":"545989b411ab3dad2dd5aa5cc55fe83943180d70"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"dcce78804b327389715e6cc934a5629ba2468088","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"739b629c_d1dfc639","updated":"2025-01-08 16:59:18.000000000","message":"recheck i suspect this was just a slow node., theer were no fialing test when it timed out so lets see if this repeats","commit_id":"99814b280fbdf8169d291e003cf6dc5a80e08074"}],"vif_plug_ovs/ovs.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"c2788606da7e60c8254211e2b8d3bbd7ab444a55","unresolved":true,"context_lines":[{"line_number":208,"context_line":"        # bound the interface in the vif binding details so isolation"},{"line_number":209,"context_line":"        # can be enabled automatically in the future."},{"line_number":210,"context_line":"        bridge \u003d kwargs.pop(\u0027bridge\u0027, vif.network.bridge)"},{"line_number":211,"context_line":"        if self._isolate_vif(vif_name, bridge):"},{"line_number":212,"context_line":"            # See bug #2069543."},{"line_number":213,"context_line":"            kwargs[\u0027tag\u0027] \u003d constants.DEAD_VLAN"},{"line_number":214,"context_line":"            kwargs[\u0027vlan_mode\u0027] \u003d \u0027trunk\u0027"}],"source_content_type":"text/x-python","patch_set":1,"id":"54c75773_051331a1","line":211,"updated":"2024-06-28 13:45:39.000000000","message":"i need to check the cod but the orginal intention was that this would only be applied to the port we add to the br-int\n\nand woudl never be applied to the tap device when its added the the br-ex\n\nthe dead valn would be added to the patch port on the brint side and be updated in the case.\n\nso i feel like either we regressed that in os-vif or we regressed that in neutron\n\ni dont have time to dig into this todya but i would like to udner stand what has change and the current behvior more fully before proceeding.","commit_id":"f65641526433866985f2047b37c672662f7b1bea"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"32f7f8ecca7bfecf103794b5e475d5f3ae000521","unresolved":true,"context_lines":[{"line_number":208,"context_line":"        # bound the interface in the vif binding details so isolation"},{"line_number":209,"context_line":"        # can be enabled automatically in the future."},{"line_number":210,"context_line":"        bridge \u003d kwargs.pop(\u0027bridge\u0027, vif.network.bridge)"},{"line_number":211,"context_line":"        if self._isolate_vif(vif_name, bridge):"},{"line_number":212,"context_line":"            # See bug #2069543."},{"line_number":213,"context_line":"            kwargs[\u0027tag\u0027] \u003d constants.DEAD_VLAN"},{"line_number":214,"context_line":"            kwargs[\u0027vlan_mode\u0027] \u003d \u0027trunk\u0027"}],"source_content_type":"text/x-python","patch_set":1,"id":"8e121ce4_b7bb3bbe","line":211,"in_reply_to":"54c75773_051331a1","updated":"2024-06-28 13:53:33.000000000","message":"a better approch woudl be to use the code added in \n\nhttps://github.com/openstack/os-vif/commit/75b290fb2a8f706583e0c12c5c5a4c0fc80e6481\n\na better approch might be to detect if tis a trunk brindge an not set the dead vlan on the tap in this case.\n\ni.e. only enabel the isolaition if its not a trun brige https://github.com/openstack/os-vif/blob/master/vif_plug_ovs/ovs.py#L409-L410","commit_id":"f65641526433866985f2047b37c672662f7b1bea"},{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"working for Ericsson, UTC+1 (+DST)"},"change_message_id":"f0f79b8746e2ca810d49be80ee7a1a8fecad5315","unresolved":false,"context_lines":[{"line_number":208,"context_line":"        # bound the interface in the vif binding details so isolation"},{"line_number":209,"context_line":"        # can be enabled automatically in the future."},{"line_number":210,"context_line":"        bridge \u003d kwargs.pop(\u0027bridge\u0027, vif.network.bridge)"},{"line_number":211,"context_line":"        if self._isolate_vif(vif_name, bridge):"},{"line_number":212,"context_line":"            # See bug #2069543."},{"line_number":213,"context_line":"            kwargs[\u0027tag\u0027] \u003d constants.DEAD_VLAN"},{"line_number":214,"context_line":"            kwargs[\u0027vlan_mode\u0027] \u003d \u0027trunk\u0027"}],"source_content_type":"text/x-python","patch_set":1,"id":"e4fd54dc_370e00ec","line":211,"in_reply_to":"8e121ce4_b7bb3bbe","updated":"2024-07-01 11:52:44.000000000","message":"Thanks for the comments! Here\u0027s an approach based on your suggestion to not add the tap in a trunk to the dead vlan.\n\nHowever I still believe we have found multiple related problems which should be fixed alongside with the original bugs, please see the updated commit message.","commit_id":"f65641526433866985f2047b37c672662f7b1bea"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"4af64a7e702f1aa43c6d1f2ba4c838eb1efb0d01","unresolved":true,"context_lines":[{"line_number":211,"context_line":"        # See bug #2069543."},{"line_number":212,"context_line":"        if (self._isolate_vif(vif_name, bridge) and"},{"line_number":213,"context_line":"                not self._is_trunk_bridge(bridge)):"},{"line_number":214,"context_line":"            kwargs[\u0027tag\u0027] \u003d constants.DEAD_VLAN"},{"line_number":215,"context_line":"            kwargs[\u0027vlan_mode\u0027] \u003d \u0027trunk\u0027"},{"line_number":216,"context_line":"            kwargs[\u0027trunks\u0027] \u003d constants.DEAD_VLAN"},{"line_number":217,"context_line":"        qos_type \u003d self._get_qos_type(vif)"}],"source_content_type":"text/x-python","patch_set":2,"id":"22697976_3ab9efaf","line":214,"range":{"start_line":214,"start_character":0,"end_line":214,"end_character":2},"updated":"2024-08-27 14:29:56.000000000","message":"per the ovs docs \n\ntag: optional integer, in range 0 to 4,095\nFor an access port, the port’s implicitly tagged VLAN. For a native-tagged or native-untagged\nport, the port’s native VLAN. Must be empty if this is a trunk port.\n\nso if we set vlan_mode \u003d trunk we must not set tag.\n\nhttp://www.openvswitch.org//ovs-vswitchd.conf.db.5.pdf","commit_id":"09447517c4a64ee07eb2a4089dd6a04fca569368"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"89da11db5e9941631766f822732c04a556a2ea1d","unresolved":true,"context_lines":[{"line_number":211,"context_line":"        # See bug #2069543."},{"line_number":212,"context_line":"        if (self._isolate_vif(vif_name, bridge) and"},{"line_number":213,"context_line":"                not self._is_trunk_bridge(bridge)):"},{"line_number":214,"context_line":"            kwargs[\u0027tag\u0027] \u003d constants.DEAD_VLAN"},{"line_number":215,"context_line":"            kwargs[\u0027vlan_mode\u0027] \u003d \u0027trunk\u0027"},{"line_number":216,"context_line":"            kwargs[\u0027trunks\u0027] \u003d constants.DEAD_VLAN"},{"line_number":217,"context_line":"        qos_type \u003d self._get_qos_type(vif)"}],"source_content_type":"text/x-python","patch_set":2,"id":"61fc0d99_16386a33","line":214,"range":{"start_line":214,"start_character":0,"end_line":214,"end_character":2},"in_reply_to":"22697976_3ab9efaf","updated":"2024-08-27 14:56:02.000000000","message":"based on some irc converation the supprot for fixing the vlan mode and trunks section was merged in neutron in zed\n\ngiven that it sould be possible to backport this to all stable branches\n\nto that end we can remove setting tag adn instead use \n\n\nkwargs[\u0027vlan_mode\u0027] \u003d \u0027trunk\u0027\nkwargs[\u0027trunks\u0027] \u003d constants.DEAD_VLAN\n\ni think we can proceed with this given the security impact\n\nif the current vlan tagging is not working as expected due to a change in behvior of ovs then https://bugs.launchpad.net/neutron/+bug/1734320\nhas been regressed and this change is also fixing a regression of a high severity CVE...\n\nso this is not just fixing https://bugs.launchpad.net/neutron/+bug/2069543","commit_id":"09447517c4a64ee07eb2a4089dd6a04fca569368"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"89b9ff942de155d66c3a29a339ae240ddf0e4e76","unresolved":false,"context_lines":[{"line_number":211,"context_line":"        # See bug #2069543."},{"line_number":212,"context_line":"        if (self._isolate_vif(vif_name, bridge) and"},{"line_number":213,"context_line":"                not self._is_trunk_bridge(bridge)):"},{"line_number":214,"context_line":"            kwargs[\u0027tag\u0027] \u003d constants.DEAD_VLAN"},{"line_number":215,"context_line":"            kwargs[\u0027vlan_mode\u0027] \u003d \u0027trunk\u0027"},{"line_number":216,"context_line":"            kwargs[\u0027trunks\u0027] \u003d constants.DEAD_VLAN"},{"line_number":217,"context_line":"        qos_type \u003d self._get_qos_type(vif)"}],"source_content_type":"text/x-python","patch_set":2,"id":"7d64dd77_11e549e6","line":214,"range":{"start_line":214,"start_character":0,"end_line":214,"end_character":2},"in_reply_to":"24ae04a9_efd6af2a","updated":"2024-11-28 13:45:54.000000000","message":"I\u0027m with Bence on this but, of course, this patch deserves a set of follow-up patches to improve the OVS agent before removing the dead vlan tag from here.\n\nThanks both for the patch and the reviews.","commit_id":"09447517c4a64ee07eb2a4089dd6a04fca569368"},{"author":{"_account_id":8313,"name":"Lajos Katona","display_name":"lajoskatona","email":"katonalala@gmail.com","username":"elajkat","status":"Ericsson Software Technology"},"change_message_id":"beb41bece0d2974732722e5b515f44b46eaee07d","unresolved":false,"context_lines":[{"line_number":211,"context_line":"        # See bug #2069543."},{"line_number":212,"context_line":"        if (self._isolate_vif(vif_name, bridge) and"},{"line_number":213,"context_line":"                not self._is_trunk_bridge(bridge)):"},{"line_number":214,"context_line":"            kwargs[\u0027tag\u0027] \u003d constants.DEAD_VLAN"},{"line_number":215,"context_line":"            kwargs[\u0027vlan_mode\u0027] \u003d \u0027trunk\u0027"},{"line_number":216,"context_line":"            kwargs[\u0027trunks\u0027] \u003d constants.DEAD_VLAN"},{"line_number":217,"context_line":"        qos_type \u003d self._get_qos_type(vif)"}],"source_content_type":"text/x-python","patch_set":2,"id":"b6bb0648_6d03a0f5","line":214,"range":{"start_line":214,"start_character":0,"end_line":214,"end_character":2},"in_reply_to":"33ccfb13_003c060e","updated":"2024-12-13 12:02:49.000000000","message":"This must be fixed in ovs-agent for sure, but as I see it will be \"duplication\" anyway as otherwise nova/os-vif should depend on neutorn-lib or such common lib","commit_id":"09447517c4a64ee07eb2a4089dd6a04fca569368"},{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"working for Ericsson, UTC+1 (+DST)"},"change_message_id":"d7feb87eb32bf8d1942914782c0cd92f4c587a95","unresolved":true,"context_lines":[{"line_number":211,"context_line":"        # See bug #2069543."},{"line_number":212,"context_line":"        if (self._isolate_vif(vif_name, bridge) and"},{"line_number":213,"context_line":"                not self._is_trunk_bridge(bridge)):"},{"line_number":214,"context_line":"            kwargs[\u0027tag\u0027] \u003d constants.DEAD_VLAN"},{"line_number":215,"context_line":"            kwargs[\u0027vlan_mode\u0027] \u003d \u0027trunk\u0027"},{"line_number":216,"context_line":"            kwargs[\u0027trunks\u0027] \u003d constants.DEAD_VLAN"},{"line_number":217,"context_line":"        qos_type \u003d self._get_qos_type(vif)"}],"source_content_type":"text/x-python","patch_set":2,"id":"86b61511_c6b26955","line":214,"range":{"start_line":214,"start_character":0,"end_line":214,"end_character":2},"in_reply_to":"61fc0d99_16386a33","updated":"2024-09-03 13:34:04.000000000","message":"\u003e per the ovs docs\n\u003e \n\u003e tag: optional integer, in range 0 to 4,095\n\u003e For an access port, the port’s implicitly tagged VLAN. For a native-tagged or native-untagged port, the port’s native VLAN. Must be empty if this is a trunk port.\n\u003e \n\u003e so if we set vlan_mode \u003d trunk we must not set tag.\n\nI started looking into not setting the \u0027tag\u0027 of dead ports (I\u0027m saying dead ports because the \"dead vlan\" is becoming quite a misnomer). I understand that neutron today uses a config combination that is directly forbidden by ovs docs. First I needed to see which valid ovs configuration could replace the current (per-the-docs) invalid ovs configuration. So I performed these tests on ovs:\n\nhttps://etherpad.opendev.org/p/os-vif-923036\n\nThis tells me that \"vlan_mode\u003dtrunk tag\u003dN trunks\u003d[N]\" behaves just like \"vlan_mode\u003dtrunk tag\u003d[] trunks\u003d[N]\". That is, ovs ignores the tag when vlan_mode is trunk and \u0027trunks\u0027 is set.\n\nSo I started looking into replacing \"vlan_mode\u003dtrunk tag\u003dN trunks\u003d[N]\" with \"vlan_mode\u003dtrunk tag\u003d[] trunks\u003d[N]\". Not just here, but in neutron as well. Until I realized that _bind_port() in ovs-agent relies on detecting a port being dead by its tag (see cur_tag and DEAD_VLAN_TAG):\n\nhttps://opendev.org/openstack/neutron/src/commit/b5c4f7df9b63b63071c67b55f74287925960ffff/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py#L1236-L1336\n\nSo we would have to change all that code just to get the same behavior as we have today. Do we really want to do that?","commit_id":"09447517c4a64ee07eb2a4089dd6a04fca569368"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"40065e4bd5c5ab2f00aae2f95c58120e04ddd6ec","unresolved":false,"context_lines":[{"line_number":211,"context_line":"        # See bug #2069543."},{"line_number":212,"context_line":"        if (self._isolate_vif(vif_name, bridge) and"},{"line_number":213,"context_line":"                not self._is_trunk_bridge(bridge)):"},{"line_number":214,"context_line":"            kwargs[\u0027tag\u0027] \u003d constants.DEAD_VLAN"},{"line_number":215,"context_line":"            kwargs[\u0027vlan_mode\u0027] \u003d \u0027trunk\u0027"},{"line_number":216,"context_line":"            kwargs[\u0027trunks\u0027] \u003d constants.DEAD_VLAN"},{"line_number":217,"context_line":"        qos_type \u003d self._get_qos_type(vif)"}],"source_content_type":"text/x-python","patch_set":2,"id":"33ccfb13_003c060e","line":214,"range":{"start_line":214,"start_character":0,"end_line":214,"end_character":2},"in_reply_to":"7d64dd77_11e549e6","updated":"2024-12-02 14:59:04.000000000","message":"After reading all the comments, I would agree with Bence that this is the right thing to do, but we should try and fix this in the ovs-agent to keep all the code dealing with this in one place.","commit_id":"09447517c4a64ee07eb2a4089dd6a04fca569368"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"05c2cc453d8b527b5a6b275d13ff7b140a7631ea","unresolved":true,"context_lines":[{"line_number":211,"context_line":"        # See bug #2069543."},{"line_number":212,"context_line":"        if (self._isolate_vif(vif_name, bridge) and"},{"line_number":213,"context_line":"                not self._is_trunk_bridge(bridge)):"},{"line_number":214,"context_line":"            kwargs[\u0027tag\u0027] \u003d constants.DEAD_VLAN"},{"line_number":215,"context_line":"            kwargs[\u0027vlan_mode\u0027] \u003d \u0027trunk\u0027"},{"line_number":216,"context_line":"            kwargs[\u0027trunks\u0027] \u003d constants.DEAD_VLAN"},{"line_number":217,"context_line":"        qos_type \u003d self._get_qos_type(vif)"}],"source_content_type":"text/x-python","patch_set":2,"id":"b2288c98_469991bd","line":214,"range":{"start_line":214,"start_character":0,"end_line":214,"end_character":2},"in_reply_to":"86b61511_c6b26955","updated":"2024-09-06 13:23:11.000000000","message":"long term I think yes. if we are going to start using trunk ports we either need to do it properly or we should not use them.\n\ncould it be done as a seperat bug fix in a diffent comiett. perhaps but i would like to hear form neturon folks\n\nwe have had downsteam escalation casue by ovs starting to enforce behavior they said was invliad in the past so we should not rely on the undocumented behavior long term.","commit_id":"09447517c4a64ee07eb2a4089dd6a04fca569368"},{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"working for Ericsson, UTC+1 (+DST)"},"change_message_id":"6460965cb28f9720f21447b4a8770c6659b4bd5b","unresolved":true,"context_lines":[{"line_number":211,"context_line":"        # See bug #2069543."},{"line_number":212,"context_line":"        if (self._isolate_vif(vif_name, bridge) and"},{"line_number":213,"context_line":"                not self._is_trunk_bridge(bridge)):"},{"line_number":214,"context_line":"            kwargs[\u0027tag\u0027] \u003d constants.DEAD_VLAN"},{"line_number":215,"context_line":"            kwargs[\u0027vlan_mode\u0027] \u003d \u0027trunk\u0027"},{"line_number":216,"context_line":"            kwargs[\u0027trunks\u0027] \u003d constants.DEAD_VLAN"},{"line_number":217,"context_line":"        qos_type \u003d self._get_qos_type(vif)"}],"source_content_type":"text/x-python","patch_set":2,"id":"b79bd7e5_b0d1782e","line":214,"range":{"start_line":214,"start_character":0,"end_line":214,"end_character":2},"in_reply_to":"b2288c98_469991bd","updated":"2024-09-12 12:54:56.000000000","message":"I can\u0027t say I\u0027m happy to touch that ovs-agent code. In my experience that\u0027s really fragile and it\u0027s very easy to introduce new bugs even with simple-looking changes. But I will follow up. However I think it would be good to do this in separate patches. What do you think about this plan?\n\n1) We merge this patch as it is (setting all three of vlan_mode, trunks and tag), because that\u0027s how neutron works for a long time and we hardly make the situation worse by doing the same here too. This would also fix the regression.\n\n2) I promise to come back soon with a pair of neutron/os-vif changes that eliminate setting tag to 4095 and relies only on setting vlan_mode and trunks. This pair of change should be a no-op in user-observable behavior, therefore I hope it would be easier to test than otherwise.","commit_id":"09447517c4a64ee07eb2a4089dd6a04fca569368"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"0d89c9fc37bd18f7da888635b424d1ba767c611d","unresolved":true,"context_lines":[{"line_number":211,"context_line":"        # See bug #2069543."},{"line_number":212,"context_line":"        if (self._isolate_vif(vif_name, bridge) and"},{"line_number":213,"context_line":"                not self._is_trunk_bridge(bridge)):"},{"line_number":214,"context_line":"            kwargs[\u0027tag\u0027] \u003d constants.DEAD_VLAN"},{"line_number":215,"context_line":"            kwargs[\u0027vlan_mode\u0027] \u003d \u0027trunk\u0027"},{"line_number":216,"context_line":"            kwargs[\u0027trunks\u0027] \u003d constants.DEAD_VLAN"},{"line_number":217,"context_line":"        qos_type \u003d self._get_qos_type(vif)"}],"source_content_type":"text/x-python","patch_set":2,"id":"24ae04a9_efd6af2a","line":214,"range":{"start_line":214,"start_character":0,"end_line":214,"end_character":2},"in_reply_to":"b79bd7e5_b0d1782e","updated":"2024-11-28 11:27:18.000000000","message":"im still not particallay happy that we are doing somethign that ovs considers invalid as that has bitten us in the past when they started to enforce it.\n\nhowever it sounds like tha twill also break neutron and be more work to fix there so ok. ill +2 this and if rodolfo is happy to proceed we can merge this.","commit_id":"09447517c4a64ee07eb2a4089dd6a04fca569368"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"72dec743ed6dc6f3296ebf16e6d3902b59f11d35","unresolved":true,"context_lines":[{"line_number":212,"context_line":"        if (self._isolate_vif(vif_name, bridge) and"},{"line_number":213,"context_line":"                not self._is_trunk_bridge(bridge)):"},{"line_number":214,"context_line":"            kwargs[\u0027tag\u0027] \u003d constants.DEAD_VLAN"},{"line_number":215,"context_line":"            kwargs[\u0027vlan_mode\u0027] \u003d \u0027trunk\u0027"},{"line_number":216,"context_line":"            kwargs[\u0027trunks\u0027] \u003d constants.DEAD_VLAN"},{"line_number":217,"context_line":"        qos_type \u003d self._get_qos_type(vif)"},{"line_number":218,"context_line":"        if qos_type is not None:"},{"line_number":219,"context_line":"            # NOTE(sean-k-mooney): If the port is not already created"}],"source_content_type":"text/x-python","patch_set":2,"id":"65489f07_383b2fe2","line":216,"range":{"start_line":215,"start_character":12,"end_line":216,"end_character":50},"updated":"2024-07-01 12:02:01.000000000","message":"why are you keeping this.\n\nwe shoudl not need to put the tap in trunk mode\nby default all prots allow all vlan tagged traffic to transit them and we certenly shoul dnot be restring it to only the dead vlan.\n\nneutron is not ment to codify the tap when it s not on the br-int so these two lines would require neutrons behavior to be changed.\n\nif we remove them we only need the os-vif change.","commit_id":"09447517c4a64ee07eb2a4089dd6a04fca569368"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"0a3b2f157edca4468f0133745bb94efcaa055713","unresolved":true,"context_lines":[{"line_number":212,"context_line":"        if (self._isolate_vif(vif_name, bridge) and"},{"line_number":213,"context_line":"                not self._is_trunk_bridge(bridge)):"},{"line_number":214,"context_line":"            kwargs[\u0027tag\u0027] \u003d constants.DEAD_VLAN"},{"line_number":215,"context_line":"            kwargs[\u0027vlan_mode\u0027] \u003d \u0027trunk\u0027"},{"line_number":216,"context_line":"            kwargs[\u0027trunks\u0027] \u003d constants.DEAD_VLAN"},{"line_number":217,"context_line":"        qos_type \u003d self._get_qos_type(vif)"},{"line_number":218,"context_line":"        if qos_type is not None:"},{"line_number":219,"context_line":"            # NOTE(sean-k-mooney): If the port is not already created"}],"source_content_type":"text/x-python","patch_set":2,"id":"3a32b70e_bb09ca4b","line":216,"range":{"start_line":215,"start_character":12,"end_line":216,"end_character":50},"in_reply_to":"1eeb0eca_7abc7f40","updated":"2024-08-26 17:50:36.000000000","message":"so explicitly_egress_direct should be disabled by default\n\nhttps://github.com/openstack/neutron/blob/e2bc6a8ef1151333b01ba855eb8efb0ecf3224ef/neutron/conf/plugins/ml2/drivers/ovs_conf.py#L227\n\nare you testing with it set to true?\n\n\ni feel like this should not be configurable in Neutron as it changes the flow semantics significantly.\n\nit would be good to get some more input form neutorn folks btu i would not expect\nvlan trunk port to be supproted with explicitly_egress_direct\u003dtrue\n\nbased on skiming https://github.com/openstack/neutron/commit/efa8dd08957b5b6b1a05f0ed412ff00462a9f216","commit_id":"09447517c4a64ee07eb2a4089dd6a04fca569368"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"18ea2019e61447601a5269c8ea231367614c87cd","unresolved":true,"context_lines":[{"line_number":212,"context_line":"        if (self._isolate_vif(vif_name, bridge) and"},{"line_number":213,"context_line":"                not self._is_trunk_bridge(bridge)):"},{"line_number":214,"context_line":"            kwargs[\u0027tag\u0027] \u003d constants.DEAD_VLAN"},{"line_number":215,"context_line":"            kwargs[\u0027vlan_mode\u0027] \u003d \u0027trunk\u0027"},{"line_number":216,"context_line":"            kwargs[\u0027trunks\u0027] \u003d constants.DEAD_VLAN"},{"line_number":217,"context_line":"        qos_type \u003d self._get_qos_type(vif)"},{"line_number":218,"context_line":"        if qos_type is not None:"},{"line_number":219,"context_line":"            # NOTE(sean-k-mooney): If the port is not already created"}],"source_content_type":"text/x-python","patch_set":2,"id":"37c5e034_43c28151","line":216,"range":{"start_line":215,"start_character":12,"end_line":216,"end_character":50},"in_reply_to":"25f1f1ae_22ae11b5","updated":"2024-08-26 14:33:13.000000000","message":"wait when did neutorn stop using the normal action for ml2/ovs\n\nthat fundemntally breaks how this feature was intended to work so yes if neutorn stop using the normal action that would break the existing os-vif logic.\n\nif that has really been changed in neutron then that is the root cause of this breakage. this feature was design to depend on using the normal actuion.","commit_id":"09447517c4a64ee07eb2a4089dd6a04fca569368"},{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"working for Ericsson, UTC+1 (+DST)"},"change_message_id":"b0e5bce6813d72b9a3be30c5fab101c2f639da73","unresolved":true,"context_lines":[{"line_number":212,"context_line":"        if (self._isolate_vif(vif_name, bridge) and"},{"line_number":213,"context_line":"                not self._is_trunk_bridge(bridge)):"},{"line_number":214,"context_line":"            kwargs[\u0027tag\u0027] \u003d constants.DEAD_VLAN"},{"line_number":215,"context_line":"            kwargs[\u0027vlan_mode\u0027] \u003d \u0027trunk\u0027"},{"line_number":216,"context_line":"            kwargs[\u0027trunks\u0027] \u003d constants.DEAD_VLAN"},{"line_number":217,"context_line":"        qos_type \u003d self._get_qos_type(vif)"},{"line_number":218,"context_line":"        if qos_type is not None:"},{"line_number":219,"context_line":"            # NOTE(sean-k-mooney): If the port is not already created"}],"source_content_type":"text/x-python","patch_set":2,"id":"25f1f1ae_22ae11b5","line":216,"range":{"start_line":215,"start_character":12,"end_line":216,"end_character":50},"in_reply_to":"2a717b92_980e23b9","updated":"2024-08-26 12:11:07.000000000","message":"Sorry for the long delay. I believe we are at a point where we cannot hold the original design assumptions valid (and our old memories of how it worked at some time in the past) and have to work with what is there in the code today. (Which kind of shows the disintegration of openstack architecture during the years, but I\u0027m not here to argue this.)\n\nBefore continuing, let me point out a few discrepancies I have discovered during the years:\n\n\u003e seting the mode to trunk shoudl not be required nor should setting\ntrunks\u003dconstants.DEAD_VLAN\n\nWe can argue about this behavior being correct or not, but I believe this change is not the place for that. Today we have this behavior and we have to work with it, unless we go back and challenge that. If you believe this behavior to be incorrect, please demonstrate its faults and propose a revert and (since that was a bugfix with a long history) a better alternative fix to this:\n\nhttps://review.opendev.org/c/openstack/neutron/+/827315\n\n\u003e you should just be installing a highest priority drop rule in the ingress table to drop all traffic on vlan 4095\n\nThis would only work if we were also using the NORMAL action, which is no longer the case. When using the output action, the vlan tag associated to an access port cannot be seen by openflow rules. For details please see:\nhttps://mail.openvswitch.org/pipermail/ovs-discuss/2021-December/051647.html\n\nand the \"My OpenFlow controller doesn’t see the VLANs that I expect.\" section of the OVS VLAN FAQ: https://docs.openvswitch.org/en/latest/faq/vlan/\n\n\u003e but neutron at some point changed the openflow rule it is generating from\nusing dl_vlan\u003d4095 to using the tci field\n\u003e we should not be using vlan_tci\u003d0x0fff/0x1fff to set the match rule\nneutron should be using dl_vlan\u003d4095, dl_vlan\u003d4095 should match any vlan packet with that vlan id regardless of the qos bits stored in the vlan_tci subfield\n\nThis was already corrected in 2022 (and I believe this part of the patch was not reverted later):\nhttps://review.opendev.org/c/openstack/neutron/+/820897\n\n\u003e we would need addtional changes in neutron and as a result we coudl not backport this change in os-vif without also backporting the neutron change in lockstep so this woul dbe a master only change\n\nAll this behavior is already backported to victoria:\nhttps://review.opendev.org/q/I0391dd24224f8656a09ddb002e7dae8783ba37a4\n\nAnyway I believe our first concern should be to fix the problem on master - and if and only if we have multiple correct approaches on master then we can choose between them based on our ability to backport.","commit_id":"09447517c4a64ee07eb2a4089dd6a04fca569368"},{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"working for Ericsson, UTC+1 (+DST)"},"change_message_id":"ba63f93f94babe8a419c893e09bcda5e06ca3255","unresolved":true,"context_lines":[{"line_number":212,"context_line":"        if (self._isolate_vif(vif_name, bridge) and"},{"line_number":213,"context_line":"                not self._is_trunk_bridge(bridge)):"},{"line_number":214,"context_line":"            kwargs[\u0027tag\u0027] \u003d constants.DEAD_VLAN"},{"line_number":215,"context_line":"            kwargs[\u0027vlan_mode\u0027] \u003d \u0027trunk\u0027"},{"line_number":216,"context_line":"            kwargs[\u0027trunks\u0027] \u003d constants.DEAD_VLAN"},{"line_number":217,"context_line":"        qos_type \u003d self._get_qos_type(vif)"},{"line_number":218,"context_line":"        if qos_type is not None:"},{"line_number":219,"context_line":"            # NOTE(sean-k-mooney): If the port is not already created"}],"source_content_type":"text/x-python","patch_set":2,"id":"57709232_cd137a88","line":216,"range":{"start_line":215,"start_character":12,"end_line":216,"end_character":50},"in_reply_to":"37c5e034_43c28151","updated":"2024-08-26 14:58:22.000000000","message":"The normal action is not completely eliminated, but neutron has direct output openflow rules for a few years. The first reason that comes to my mind is this patch series:\n\nneutron$ git log --grep explicitly_egress_direct","commit_id":"09447517c4a64ee07eb2a4089dd6a04fca569368"},{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"working for Ericsson, UTC+1 (+DST)"},"change_message_id":"d7fffe9ed5a6e6934b493f6b6084eea2159c4655","unresolved":true,"context_lines":[{"line_number":212,"context_line":"        if (self._isolate_vif(vif_name, bridge) and"},{"line_number":213,"context_line":"                not self._is_trunk_bridge(bridge)):"},{"line_number":214,"context_line":"            kwargs[\u0027tag\u0027] \u003d constants.DEAD_VLAN"},{"line_number":215,"context_line":"            kwargs[\u0027vlan_mode\u0027] \u003d \u0027trunk\u0027"},{"line_number":216,"context_line":"            kwargs[\u0027trunks\u0027] \u003d constants.DEAD_VLAN"},{"line_number":217,"context_line":"        qos_type \u003d self._get_qos_type(vif)"},{"line_number":218,"context_line":"        if qos_type is not None:"},{"line_number":219,"context_line":"            # NOTE(sean-k-mooney): If the port is not already created"}],"source_content_type":"text/x-python","patch_set":2,"id":"1eeb0eca_7abc7f40","line":216,"range":{"start_line":215,"start_character":12,"end_line":216,"end_character":50},"in_reply_to":"57709232_cd137a88","updated":"2024-08-26 15:01:53.000000000","message":"My more important other point wanted to be that the dead vlan drop rule does not work in combination with access ports (and not pushing the vlan directly onto the frame by an openflow rule).","commit_id":"09447517c4a64ee07eb2a4089dd6a04fca569368"},{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"working for Ericsson, UTC+1 (+DST)"},"change_message_id":"684271bfe4f9988b623d1b896c51e247edff3982","unresolved":true,"context_lines":[{"line_number":212,"context_line":"        if (self._isolate_vif(vif_name, bridge) and"},{"line_number":213,"context_line":"                not self._is_trunk_bridge(bridge)):"},{"line_number":214,"context_line":"            kwargs[\u0027tag\u0027] \u003d constants.DEAD_VLAN"},{"line_number":215,"context_line":"            kwargs[\u0027vlan_mode\u0027] \u003d \u0027trunk\u0027"},{"line_number":216,"context_line":"            kwargs[\u0027trunks\u0027] \u003d constants.DEAD_VLAN"},{"line_number":217,"context_line":"        qos_type \u003d self._get_qos_type(vif)"},{"line_number":218,"context_line":"        if qos_type is not None:"},{"line_number":219,"context_line":"            # NOTE(sean-k-mooney): If the port is not already created"}],"source_content_type":"text/x-python","patch_set":2,"id":"85dff25b_2aaf3298","line":216,"range":{"start_line":215,"start_character":12,"end_line":216,"end_character":50},"in_reply_to":"65489f07_383b2fe2","updated":"2024-07-02 11:49:34.000000000","message":"Have you seen these patches?\n\nhttps://review.opendev.org/c/openstack/neutron/+/827315\nhttps://review.opendev.org/c/openstack/neutron/+/820897\n\nI believe these explain why neutron sets not just the tag but vlan_mode and trunks too.","commit_id":"09447517c4a64ee07eb2a4089dd6a04fca569368"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"51f1a574b55709d632b15116937ba3471095bb8a","unresolved":true,"context_lines":[{"line_number":212,"context_line":"        if (self._isolate_vif(vif_name, bridge) and"},{"line_number":213,"context_line":"                not self._is_trunk_bridge(bridge)):"},{"line_number":214,"context_line":"            kwargs[\u0027tag\u0027] \u003d constants.DEAD_VLAN"},{"line_number":215,"context_line":"            kwargs[\u0027vlan_mode\u0027] \u003d \u0027trunk\u0027"},{"line_number":216,"context_line":"            kwargs[\u0027trunks\u0027] \u003d constants.DEAD_VLAN"},{"line_number":217,"context_line":"        qos_type \u003d self._get_qos_type(vif)"},{"line_number":218,"context_line":"        if qos_type is not None:"},{"line_number":219,"context_line":"            # NOTE(sean-k-mooney): If the port is not already created"}],"source_content_type":"text/x-python","patch_set":2,"id":"2a717b92_980e23b9","line":216,"range":{"start_line":215,"start_character":12,"end_line":216,"end_character":50},"in_reply_to":"72610429_42c188de","updated":"2024-08-06 11:59:06.000000000","message":"i guess the fact the zuul jobs worked implies that this somehow works but its not clear to me how/when the configuration on the tap is corrected today to remove\n\nkwargs[\u0027vlan_mode\u0027] \u003d \u0027trunk\u0027\nkwargs[\u0027trunks\u0027] \u003d constants.DEAD_VLAN","commit_id":"09447517c4a64ee07eb2a4089dd6a04fca569368"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"08dc8927e6a376bd35114eb2d0b63d5604371d12","unresolved":true,"context_lines":[{"line_number":212,"context_line":"        if (self._isolate_vif(vif_name, bridge) and"},{"line_number":213,"context_line":"                not self._is_trunk_bridge(bridge)):"},{"line_number":214,"context_line":"            kwargs[\u0027tag\u0027] \u003d constants.DEAD_VLAN"},{"line_number":215,"context_line":"            kwargs[\u0027vlan_mode\u0027] \u003d \u0027trunk\u0027"},{"line_number":216,"context_line":"            kwargs[\u0027trunks\u0027] \u003d constants.DEAD_VLAN"},{"line_number":217,"context_line":"        qos_type \u003d self._get_qos_type(vif)"},{"line_number":218,"context_line":"        if qos_type is not None:"},{"line_number":219,"context_line":"            # NOTE(sean-k-mooney): If the port is not already created"}],"source_content_type":"text/x-python","patch_set":2,"id":"8c385048_7a462878","line":216,"range":{"start_line":215,"start_character":12,"end_line":216,"end_character":50},"in_reply_to":"85dff25b_2aaf3298","updated":"2024-07-03 06:55:48.000000000","message":"We (Oleg) implemented this trick in Neutron [1] to prevent any ingress traffic from this new port. The rationale is explained in [1]: only VLAN tagged 4095 traffic can access and this specific VLAN ID is blocked. Without this trick, we can\u0027t prevent any other traffic (tagged or not) coming into br-int because we don\u0027t block it.\n\nOnce the port receives a valid VLAN tag (provided by the OVS agent), this configuration is removed [2].\n\n[1]https://review.opendev.org/c/openstack/neutron/+/827315\n[2]https://review.opendev.org/c/openstack/neutron/+/827315/12/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py","commit_id":"09447517c4a64ee07eb2a4089dd6a04fca569368"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"e1545a3c1764ff376b7449bcbcf249f275f12f40","unresolved":true,"context_lines":[{"line_number":212,"context_line":"        if (self._isolate_vif(vif_name, bridge) and"},{"line_number":213,"context_line":"                not self._is_trunk_bridge(bridge)):"},{"line_number":214,"context_line":"            kwargs[\u0027tag\u0027] \u003d constants.DEAD_VLAN"},{"line_number":215,"context_line":"            kwargs[\u0027vlan_mode\u0027] \u003d \u0027trunk\u0027"},{"line_number":216,"context_line":"            kwargs[\u0027trunks\u0027] \u003d constants.DEAD_VLAN"},{"line_number":217,"context_line":"        qos_type \u003d self._get_qos_type(vif)"},{"line_number":218,"context_line":"        if qos_type is not None:"},{"line_number":219,"context_line":"            # NOTE(sean-k-mooney): If the port is not already created"}],"source_content_type":"text/x-python","patch_set":2,"id":"cb4c1c07_e1d000f1","line":216,"range":{"start_line":215,"start_character":12,"end_line":216,"end_character":50},"in_reply_to":"85dff25b_2aaf3298","updated":"2024-07-03 11:54:54.000000000","message":"if i recall correctly when i was testing the isolation feature in the past i explictly tested that packs were dropped proably using secure mode since that is the default in devstack/neutron if i am rembering correctly.\n\nits possibel if you had it set to standalone or unset the behaivor was different.\nits also possibel that since https://bugs.launchpad.net/neutron/+bug/1734320 was first fixe the role were modifed such that it was regress requireing those addtional fixes.\n\ni would have to dig into those more to understand why they were required when they did not appare to be requried when we initally tested the cve mitigation.\n\ni also want to point out that we only run this code if the prot does nto already exist to avoid impacting the vms network connectiviy if nova-comptue is restarted.\n\nso    \n\nkwargs[\u0027vlan_mode\u0027] \u003d \u0027trunk\u0027\nkwargs[\u0027trunks\u0027] \u003d constants.DEAD_VLAN\n\nwould not be applied to any existing port and neutron would need to cater for that.\n\n@ralonsoh@redhat.com entering the br-int is fine leaving it vai any means would not be. i do not belive https://review.opendev.org/c/openstack/neutron/+/827315/12/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py#1238 is correct\n\nyou should just be installing a highest priority drop rule in the ingress table to drop all traffic on vlan 4095\n\n[stack@c9s-devstack devstack]$ sudo ovs-ofctl add-flow test priority\u003d65535,dl_vlan\u003d4095,action\u003ddrop\n[stack@c9s-devstack devstack]$ sudo ovs-ofctl dump-flows test\n cookie\u003d0x0, duration\u003d10.249s, table\u003d0, n_packets\u003d0, n_bytes\u003d0, priority\u003d65535,dl_vlan\u003d4095 actions\u003ddrop\n cookie\u003d0x0, duration\u003d231.629s, table\u003d0, n_packets\u003d0, n_bytes\u003d0, priority\u003d0 actions\u003dNORMAL\n\nwe should not be using\n\nvlan_tci\u003d0x0fff/0x1fff to set the match rule\nneutron should be using dl_vlan\u003d4095,  dl_vlan\u003d4095 should match any vlan packet with that vlan id regardless of the qos bits stored in the vlan_tci subfield.","commit_id":"09447517c4a64ee07eb2a4089dd6a04fca569368"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"77ed8f0a75c25c35020c8ddee6625c9fc1f3b2db","unresolved":true,"context_lines":[{"line_number":212,"context_line":"        if (self._isolate_vif(vif_name, bridge) and"},{"line_number":213,"context_line":"                not self._is_trunk_bridge(bridge)):"},{"line_number":214,"context_line":"            kwargs[\u0027tag\u0027] \u003d constants.DEAD_VLAN"},{"line_number":215,"context_line":"            kwargs[\u0027vlan_mode\u0027] \u003d \u0027trunk\u0027"},{"line_number":216,"context_line":"            kwargs[\u0027trunks\u0027] \u003d constants.DEAD_VLAN"},{"line_number":217,"context_line":"        qos_type \u003d self._get_qos_type(vif)"},{"line_number":218,"context_line":"        if qos_type is not None:"},{"line_number":219,"context_line":"            # NOTE(sean-k-mooney): If the port is not already created"}],"source_content_type":"text/x-python","patch_set":2,"id":"72610429_42c188de","line":216,"range":{"start_line":215,"start_character":12,"end_line":216,"end_character":50},"in_reply_to":"cb4c1c07_e1d000f1","updated":"2024-08-06 11:57:37.000000000","message":"im not quite sure where we are with this at this point.\n\nseting the mode to trunk shoudl not be required nor should setting \ntrunks\u003dconstants.DEAD_VLAN\n\nbut neutron at some point changed the openflow rule it is generating from \nusing dl_vlan\u003d4095 to using the tci field.\n\nthat regression likely happend several years ago when moving to the python based binding or perhaps in a later refactoring and went un noticed.\n\nmy understandign is if we are setting\n```\nkwargs[\u0027vlan_mode\u0027] \u003d \u0027trunk\u0027\nkwargs[\u0027trunks\u0027] \u003d constants.DEAD_VLAN\n```\nwe would need addtional changes in neutron and as a result we coudl not backport this change in os-vif without also backporting the neutron change in lockstep so this woul dbe a master only change.\n\n\nif we dont set \n```\nkwargs[\u0027vlan_mode\u0027] \u003d \u0027trunk\u0027\nkwargs[\u0027trunks\u0027] \u003d constants.DEAD_VLAN\n```\n\ni think this change can be backported in os-vif only without any other changes in neutron?\n\nis that correct.","commit_id":"09447517c4a64ee07eb2a4089dd6a04fca569368"},{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"working for Ericsson, UTC+1 (+DST)"},"change_message_id":"aaa8fb9da25579ae060abc5704490ac30a5aed3f","unresolved":false,"context_lines":[{"line_number":307,"context_line":""},{"line_number":308,"context_line":"        self.ovsdb.ensure_ovs_bridge("},{"line_number":309,"context_line":"             int_bridge_name, self._get_vif_datapath_type(vif))"},{"line_number":310,"context_line":"        self.ovsdb.ensure_ovs_bridge("},{"line_number":311,"context_line":"            port_bridge_name, self._get_vif_datapath_type(vif))"},{"line_number":312,"context_line":"        self._create_vif_port("},{"line_number":313,"context_line":"            vif, vif.vif_name, instance_info, bridge\u003dport_bridge_name,"}],"source_content_type":"text/x-python","patch_set":4,"id":"ba92f199_3dd399c9","line":310,"in_reply_to":"2441ba85_98b7a157","updated":"2025-01-08 12:00:56.000000000","message":"\u003e pep8: E231 missing whitespace after \u0027:\u0027\n\nThis error message should actually refer to this line:\nhttps://opendev.org/openstack/os-vif/src/commit/71521590da560bd32665b459e70af90b9fff0e3c/vif_plug_ovs/ovs.py#L306\n\nWhich is of course not a syntax error, however flake8 3.7.9 believes it is. Newer flake8 versions do not. We use that flake8 version because hacking is pinned in tox.ini:\nhttps://opendev.org/openstack/os-vif/src/commit/71521590da560bd32665b459e70af90b9fff0e3c/tox.ini#L61\nhttps://opendev.org/openstack/hacking/src/commit/d30425b3d89ed47c9e19f17062d74ed395353a4d/requirements.txt#L1\n\nSince Stephen already pushed a change to move to pre-commit which removes the hacking pin I rebased this change on top of Stephen\u0027s:\nhttps://review.opendev.org/c/openstack/os-vif/+/934240","commit_id":"545989b411ab3dad2dd5aa5cc55fe83943180d70"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"9b4357cb7dbfe36eefc95dee7af593e678c8860d","unresolved":false,"context_lines":[{"line_number":307,"context_line":""},{"line_number":308,"context_line":"        self.ovsdb.ensure_ovs_bridge("},{"line_number":309,"context_line":"             int_bridge_name, self._get_vif_datapath_type(vif))"},{"line_number":310,"context_line":"        self.ovsdb.ensure_ovs_bridge("},{"line_number":311,"context_line":"            port_bridge_name, self._get_vif_datapath_type(vif))"},{"line_number":312,"context_line":"        self._create_vif_port("},{"line_number":313,"context_line":"            vif, vif.vif_name, instance_info, bridge\u003dport_bridge_name,"}],"source_content_type":"text/x-python","patch_set":4,"id":"4e4e16b5_d4fee39f","line":310,"in_reply_to":"ba92f199_3dd399c9","updated":"2025-01-08 13:40:33.000000000","message":"i appoved stephens change to unblock this.","commit_id":"545989b411ab3dad2dd5aa5cc55fe83943180d70"}],"vif_plug_ovs/ovsdb/ovsdb_lib.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"e559869f80c411bd1579f480e2282542e76889b4","unresolved":true,"context_lines":[{"line_number":211,"context_line":"            if tag:"},{"line_number":212,"context_line":"                txn.add(self.ovsdb.db_set(\u0027Port\u0027, dev, (\u0027tag\u0027, tag)))"},{"line_number":213,"context_line":"            if vlan_mode:"},{"line_number":214,"context_line":"                txn.add(self.ovsdb.db_set(\u0027Port\u0027, dev, (\u0027vlan_mode\u0027, vlan_mode)))"},{"line_number":215,"context_line":"            if trunks:"},{"line_number":216,"context_line":"                txn.add(self.ovsdb.db_set(\u0027Port\u0027, dev, (\u0027trunks\u0027, trunks)))"},{"line_number":217,"context_line":"            if qid:"}],"source_content_type":"text/x-python","patch_set":1,"id":"75421809_8ee7598b","line":214,"updated":"2024-06-28 13:15:19.000000000","message":"we intentionlaly do not use vlan_mode trunk in ovs.\n\ni assume that there is soem work happenign to change how the datapath is cofnigured in neutorn?","commit_id":"f65641526433866985f2047b37c672662f7b1bea"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"e1545a3c1764ff376b7449bcbcf249f275f12f40","unresolved":true,"context_lines":[{"line_number":214,"context_line":"                txn.add(self.ovsdb.db_set(\u0027Port\u0027, dev,"},{"line_number":215,"context_line":"                                          (\u0027vlan_mode\u0027, vlan_mode)))"},{"line_number":216,"context_line":"            if trunks:"},{"line_number":217,"context_line":"                txn.add(self.ovsdb.db_set(\u0027Port\u0027, dev, (\u0027trunks\u0027, trunks)))"},{"line_number":218,"context_line":"            if qid:"},{"line_number":219,"context_line":"                txn.add(self.ovsdb.db_set(\u0027Port\u0027, dev, (\u0027qos\u0027, qid)))"},{"line_number":220,"context_line":"            if col_values:"}],"source_content_type":"text/x-python","patch_set":3,"id":"84be0e8f_4e55ca80","line":217,"updated":"2024-07-03 11:54:54.000000000","message":"if we keep this change you shoudl also have functional tests for this change not just unit tests.","commit_id":"9a88608e89745dba8a4857eca00c8c711c3247aa"},{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"working for Ericsson, UTC+1 (+DST)"},"change_message_id":"74daafe1ea6c3e338dc2b2499f969968f30eed16","unresolved":false,"context_lines":[{"line_number":214,"context_line":"                txn.add(self.ovsdb.db_set(\u0027Port\u0027, dev,"},{"line_number":215,"context_line":"                                          (\u0027vlan_mode\u0027, vlan_mode)))"},{"line_number":216,"context_line":"            if trunks:"},{"line_number":217,"context_line":"                txn.add(self.ovsdb.db_set(\u0027Port\u0027, dev, (\u0027trunks\u0027, trunks)))"},{"line_number":218,"context_line":"            if qid:"},{"line_number":219,"context_line":"                txn.add(self.ovsdb.db_set(\u0027Port\u0027, dev, (\u0027qos\u0027, qid)))"},{"line_number":220,"context_line":"            if col_values:"}],"source_content_type":"text/x-python","patch_set":3,"id":"d4f0bbf1_cec9a2e8","line":217,"in_reply_to":"84be0e8f_4e55ca80","updated":"2024-07-04 14:44:00.000000000","message":"Done","commit_id":"9a88608e89745dba8a4857eca00c8c711c3247aa"}]}