)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":16137,"name":"Tobias Urdin","email":"tobias.urdin@binero.com","username":"tobasco"},"change_message_id":"68a461f9480c5575bf38e064601c398e786c7302","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"c02075cb_1b4c39bc","updated":"2024-06-28 10:18:04.000000000","message":"see my comment in the bug https://bugs.launchpad.net/neutron/+bug/2069543 – this is probably going to have race condition issues","commit_id":"20b1f79ab33c5f2271618260a7e95ce505529aa0"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"085349a6c492752a6bcfa214d4f0810985089850","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"e139fd82_e74dd04f","updated":"2024-06-28 13:55:19.000000000","message":"this sould not be requried\n\nneutron shoudl not need to update the vlan on the tap as we are not ment to set it in the first place.\n\nthe isolated_vif secuirty featre was only ment to aplly the dead vlan to port added to the br-int","commit_id":"20b1f79ab33c5f2271618260a7e95ce505529aa0"},{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"working for Ericsson, UTC+1 (+DST)"},"change_message_id":"88ce1677ff75ae07c2d06ca3fcd6755e6fdbc94b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"4adb8bf0_510e0497","in_reply_to":"c02075cb_1b4c39bc","updated":"2024-07-01 11:54:03.000000000","message":"Thanks, I did not notice that.\n\nBased on this and Sean\u0027s comments on the os-vif counterpart I revised these patches completely.","commit_id":"20b1f79ab33c5f2271618260a7e95ce505529aa0"},{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"working for Ericsson, UTC+1 (+DST)"},"change_message_id":"051f648c2ec57b482a53f9bb97c0d5133514dead","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"3427ec43_fd9cad70","updated":"2024-07-04 10:41:59.000000000","message":"In the review comments of https://review.opendev.org/c/openstack/os-vif/+/923036 we discussed whether this is needed or not. As a result I first abandoned this. Then restored it, because I\u0027m still not sure.\n\nBasically I noticed a possible problem while working on that other bug. But so far I did not demonstrate a separate bug. Anyway the possible problem stems from the fact that for trunk ports we neither set the final vlan on tpi/spi ports in the create transaction, nor put them in the dead vlan. Therefore there is a time window in which traffic can get through these ports while they are not in their proper vlans. Therefore we may have traffic leaking. But at the moment I cannot demonstrate that this problem can be exploited in any way.\n\nPlease consider this output while creating a trunk port:\n\n    $ sudo ovsdb-client monitor Port name tag trunks vlan_mode\n    row                                  action name           tag trunks vlan_mode\n    ------------------------------------ ------ -------------- --- ------ ---------\n    9b779622-afc7-4f55-ab46-565f340907a4 insert tbr-a375bc09-7 []  []     []\n    \n    row                                  action name           tag trunks vlan_mode\n    ------------------------------------ ------ -------------- --- ------ ---------\n    1677a4c1-9a91-4618-98b0-e5095ceb87ef insert tapcf2856f9-65 []  []     []\n    \n    row                                  action name            tag trunks vlan_mode\n    ------------------------------------ ------ --------------- --- ------ ---------\n    cc26ac2c-3b6c-44ee-8d9d-46d8b8ddae01 insert tpi-cf2856f9-65 []  []     []\n    96a28f1d-0b79-47c3-8c88-ccef4d5502e2 insert tpt-cf2856f9-65 0   []     access\n    \n    row                                  action name            tag trunks vlan_mode\n    ------------------------------------ ------ --------------- --- ------ ---------\n    a68aa8f2-62dd-4d62-https://opendev.org/openstack/neutron/src/commit/cfab008eef3d11055ad16b8dd0d49aaa1801bc95/neutron/services/trunk/drivers/openvswitch/agent/trunk_manager.py#L118-L129980d-eb2df88c82bf insert spi-7914d497-b6 []  []     []\n    8dc56e92-7403-4b99-8593-54f701c9c8a4 insert spt-7914d497-b6 101 []     access\n    \n    row                                  action name            tag trunks vlan_mode\n    ------------------------------------ ------ --------------- --- ------ ---------\n    a68aa8f2-62dd-4d62-980d-eb2df88c82bf old                    []\n                                         new    spi-7914d497-b6 2   []     []\n    \n    row                                  action name            tag trunks vlan_mode\n    ------------------------------------ ------ --------------- --- ------ ---------\n    cc26ac2c-3b6c-44ee-8d9d-46d8b8ddae01 old                    []\n                                         new    tpi-cf2856f9-65 1   []     []\n                                         \nAlso consider the following code:\nhttps://opendev.org/openstack/neutron/src/commit/cfab008eef3d11055ad16b8dd0d49aaa1801bc95/neutron/services/trunk/drivers/openvswitch/agent/trunk_manager.py#L118-L129\n\nWhile \u0027ovsdb-client monitor\u0027 does not show transaction boundaries, I believe it\u0027s clear that in the port create transaction we do not set the final tag of the port, but an intermediate tag (for example for tpi ports it\u0027s always tag\u003d0).\n\nWhile theoretically neutron could set the final tag in the create transaction, I don\u0027t see a way to do that in the current implementation, because we intentionally separated the trunk-specific from the non-trunk-specific code and we need both to create the tpi/spi ports and to put them into their final vlan. Therefore this patch proposes to first put these ports in the dead vlan, just like we put other ports in br-int into the dead vlan before thay can be set up properly.","commit_id":"b6604d809f9a4efb54eeece53ba43cf1d4a4c2c9"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"aab1fca556314c57aa25e70d1b1c1fd2cff59b89","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"6a0e3913_da0123f7","updated":"2024-07-01 12:16:44.000000000","message":"this is not requried.\n\nthe reason we use the dead vlan in the first palce is os-vif does not know what is the corect vlan to use as this si a value that is geenrated in the vlan manager code internally in the l2 agent\n\nthere is never a valid reason for the l2 agent to place a port in the dead vlan  as far as i am aware and since the tpi and spi patch ports are created by the l2 agent these should never need to be in the dead vlan.\n\nneutron know what vlan they should be part of when its creating them","commit_id":"b6604d809f9a4efb54eeece53ba43cf1d4a4c2c9"}]}