)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"5005e7296cac8bf6f6032f1f2b1cfa3904caa12f","unresolved":true,"context_lines":[{"line_number":22,"context_line":"When the VM requests metadata from its LLA, the traffic will reach"},{"line_number":23,"context_line":"the ovnmeta namespace associated to its network. This patch also"},{"line_number":24,"context_line":"uses a \u0027/64\u0027 (vs /128) CIDR for the metadata IPv6 address so that"},{"line_number":25,"context_line":"it can communicate to the VIF without adding any static routes."},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"Change-Id: Idcef6de33ed2a73cb3c426db1c55fa9cd06de63f"},{"line_number":28,"context_line":"Signed-off-by: Daniel Alvarez Sanchez \u003cdalvarez@redhat.com\u003e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"9432e577_c695adcf","line":25,"range":{"start_line":25,"start_character":1,"end_line":25,"end_character":62},"updated":"2023-09-07 15:20:10.000000000","message":"I have a question related to this address. If we have two compute nodes, both with the metadata namespace created, both will have the same interface with the same IPv6 address. That could lead to a DAD failure (same as in OVS).\n\nIf the VM interface should send the traffic to this specific IPv6, what interface will the traffic go trough?","commit_id":"fb154c6663b6b60fd417511c0100847d0202d44b"},{"author":{"_account_id":23804,"name":"Daniel Alvarez","email":"dalvarez@redhat.com","username":"dalvarez"},"change_message_id":"36ed3205a0d299eb0c88cce8f52be0e3f4fa3df7","unresolved":true,"context_lines":[{"line_number":22,"context_line":"When the VM requests metadata from its LLA, the traffic will reach"},{"line_number":23,"context_line":"the ovnmeta namespace associated to its network. This patch also"},{"line_number":24,"context_line":"uses a \u0027/64\u0027 (vs /128) CIDR for the metadata IPv6 address so that"},{"line_number":25,"context_line":"it can communicate to the VIF without adding any static routes."},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"Change-Id: Idcef6de33ed2a73cb3c426db1c55fa9cd06de63f"},{"line_number":28,"context_line":"Signed-off-by: Daniel Alvarez Sanchez \u003cdalvarez@redhat.com\u003e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"b4b2516c_584864e5","line":25,"range":{"start_line":25,"start_character":1,"end_line":25,"end_character":62},"in_reply_to":"2935fb78_3d53f69e","updated":"2023-09-07 15:29:53.000000000","message":"The namespace that contains the interface with that IPv6 address is connected to br-int via an OVN localport. By definition, OVN localports won\u0027t send traffic out of the hypervisor so this scenario that you mention is not possible.\n\nFor the very same reason, we have always had the metadata port with the same IPv4 address configured on every hypervisor and we never experienced DAD failures.\n\nDoes this make sense?","commit_id":"fb154c6663b6b60fd417511c0100847d0202d44b"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"941cad2c571d7fbd8babfdc656e79b59068b4f17","unresolved":true,"context_lines":[{"line_number":22,"context_line":"When the VM requests metadata from its LLA, the traffic will reach"},{"line_number":23,"context_line":"the ovnmeta namespace associated to its network. This patch also"},{"line_number":24,"context_line":"uses a \u0027/64\u0027 (vs /128) CIDR for the metadata IPv6 address so that"},{"line_number":25,"context_line":"it can communicate to the VIF without adding any static routes."},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"Change-Id: Idcef6de33ed2a73cb3c426db1c55fa9cd06de63f"},{"line_number":28,"context_line":"Signed-off-by: Daniel Alvarez Sanchez \u003cdalvarez@redhat.com\u003e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"2935fb78_3d53f69e","line":25,"range":{"start_line":25,"start_character":1,"end_line":25,"end_character":62},"in_reply_to":"9432e577_c695adcf","updated":"2023-09-07 15:21:02.000000000","message":"(now I refreshed the review, I see Brian\u0027s comment, similar to mine)","commit_id":"fb154c6663b6b60fd417511c0100847d0202d44b"},{"author":{"_account_id":23804,"name":"Daniel Alvarez","email":"dalvarez@redhat.com","username":"dalvarez"},"change_message_id":"0a79d4508e605fec9fc28e4787ad2a0d71dc9801","unresolved":false,"context_lines":[{"line_number":22,"context_line":"When the VM requests metadata from its LLA, the traffic will reach"},{"line_number":23,"context_line":"the ovnmeta namespace associated to its network. This patch also"},{"line_number":24,"context_line":"uses a \u0027/64\u0027 (vs /128) CIDR for the metadata IPv6 address so that"},{"line_number":25,"context_line":"it can communicate to the VIF without adding any static routes."},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"Change-Id: Idcef6de33ed2a73cb3c426db1c55fa9cd06de63f"},{"line_number":28,"context_line":"Signed-off-by: Daniel Alvarez Sanchez \u003cdalvarez@redhat.com\u003e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"d38d6efb_7f417d87","line":25,"range":{"start_line":25,"start_character":1,"end_line":25,"end_character":62},"in_reply_to":"a27f61c1_7f381712","updated":"2023-09-13 16:00:04.000000000","message":"Done","commit_id":"fb154c6663b6b60fd417511c0100847d0202d44b"},{"author":{"_account_id":23804,"name":"Daniel Alvarez","email":"dalvarez@redhat.com","username":"dalvarez"},"change_message_id":"3775c96f9bbad28902809b6c83d748d81f0b9b20","unresolved":true,"context_lines":[{"line_number":22,"context_line":"When the VM requests metadata from its LLA, the traffic will reach"},{"line_number":23,"context_line":"the ovnmeta namespace associated to its network. This patch also"},{"line_number":24,"context_line":"uses a \u0027/64\u0027 (vs /128) CIDR for the metadata IPv6 address so that"},{"line_number":25,"context_line":"it can communicate to the VIF without adding any static routes."},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"Change-Id: Idcef6de33ed2a73cb3c426db1c55fa9cd06de63f"},{"line_number":28,"context_line":"Signed-off-by: Daniel Alvarez Sanchez \u003cdalvarez@redhat.com\u003e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"a27f61c1_7f381712","line":25,"range":{"start_line":25,"start_character":1,"end_line":25,"end_character":62},"in_reply_to":"ae48564e_d0496562","updated":"2023-09-07 20:37:07.000000000","message":"I haven\u0027t deployed with multiple nodes but I couldn\u0027t see the NS anywhere else.\nRodolfo was going to deploy with more than one compute node.","commit_id":"fb154c6663b6b60fd417511c0100847d0202d44b"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"acf01506d159538622220e17eaf5a4be106b382b","unresolved":true,"context_lines":[{"line_number":22,"context_line":"When the VM requests metadata from its LLA, the traffic will reach"},{"line_number":23,"context_line":"the ovnmeta namespace associated to its network. This patch also"},{"line_number":24,"context_line":"uses a \u0027/64\u0027 (vs /128) CIDR for the metadata IPv6 address so that"},{"line_number":25,"context_line":"it can communicate to the VIF without adding any static routes."},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"Change-Id: Idcef6de33ed2a73cb3c426db1c55fa9cd06de63f"},{"line_number":28,"context_line":"Signed-off-by: Daniel Alvarez Sanchez \u003cdalvarez@redhat.com\u003e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"ae48564e_d0496562","line":25,"range":{"start_line":25,"start_character":1,"end_line":25,"end_character":62},"in_reply_to":"b4b2516c_584864e5","updated":"2023-09-07 16:42:40.000000000","message":"An IPv4 DAD failure is non-fatal, and most likely happens with OVS today (multiple dhcp-agents and isolated metadata case). In the IPv6 case it\u0027s fatal because it enters the \"dadfailed\" state. But if the Neighbour Solicitation for the address is never seen on other ports then I guess you\u0027ll never see that?","commit_id":"fb154c6663b6b60fd417511c0100847d0202d44b"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"5bbe7b69b7a9c646e3510d9edeb4d1715429b363","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"4cc8a46e_b89d38b6","updated":"2023-09-07 14:12:36.000000000","message":"I think there might be something in the OVN gap document about this which can be removed.","commit_id":"fb154c6663b6b60fd417511c0100847d0202d44b"},{"author":{"_account_id":23804,"name":"Daniel Alvarez","email":"dalvarez@redhat.com","username":"dalvarez"},"change_message_id":"5fdb29b7db2f8187960459e2a2ad62db60729c4d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"fc7433e3_63cd5eb1","updated":"2023-09-07 15:26:50.000000000","message":"Thanks a lot!!\nAlso, tested this locally and passes tempest:\n\n(tempest) stack@devstack:~/tempest$ tempest run --serial --regex \u0027(^neutron_tempest_plugin\\.scenario.test_metadata)\u0027\n{0} neutron_tempest_plugin.scenario.test_metadata.MetadataTest.test_metadata_routed [52.024876s] ... ok","commit_id":"fb154c6663b6b60fd417511c0100847d0202d44b"},{"author":{"_account_id":23804,"name":"Daniel Alvarez","email":"dalvarez@redhat.com","username":"dalvarez"},"change_message_id":"5fdb29b7db2f8187960459e2a2ad62db60729c4d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"975ecca5_90504052","in_reply_to":"4cc8a46e_b89d38b6","updated":"2023-09-07 15:26:50.000000000","message":"Thanks Brian, totally right! I\u0027ll fix this once I get some more reviews","commit_id":"fb154c6663b6b60fd417511c0100847d0202d44b"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"d351d8eb4e5dfa6617128378b5ba4d60b072a6e5","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"03860213_b010e0bf","updated":"2023-09-11 13:06:25.000000000","message":"As commented in [1], I\u0027ve tested this feature and works, even with several compute nodes with the same IPv6 address.\n\n-1 for:\n* Release note.\n* Documentation.\n* Reference to the n-t-p patch enabling the testing for this feature in OVN.\n\n[1]https://review.opendev.org/c/openstack/neutron/+/894026/comment/a967ec56_bd2820f1/","commit_id":"5b2f37a1b851f635eab020e18981370ba130db0b"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"24316f84351cdc3fc044c279c670325ddedc1860","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"6fa57fc0_3cba6aa9","updated":"2023-09-08 21:30:56.000000000","message":"So I threw out this as an option:\n\nhttps://review.opendev.org/c/openstack/neutron/+/894399\n\nIt doesn\u0027t work at the moment, but does move towards IPv6 support, and removes a lot of duplicated code.\n\nI\u0027ll take a closer look next week.","commit_id":"5b2f37a1b851f635eab020e18981370ba130db0b"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"6915e1cdfac56afaafa9a1388e2d7beac3df71a9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"f350282f_853cac62","in_reply_to":"6fa57fc0_3cba6aa9","updated":"2023-09-13 09:46:05.000000000","message":"I agree with refactoring both metadata services and combine the code. But first I would merge this patch that closes the parity gap, providing ipv6 metadata for OVN. Once merged and tested (there is a n-t-p testing it), I would go for the refactor.","commit_id":"5b2f37a1b851f635eab020e18981370ba130db0b"},{"author":{"_account_id":23804,"name":"Daniel Alvarez","email":"dalvarez@redhat.com","username":"dalvarez"},"change_message_id":"50c477d21047645fcc0607f96af1d12f30a535c8","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"685b2a00_7c78a0d6","updated":"2023-09-13 15:39:01.000000000","message":"Thanks a lot for the reviews.\nThanks Rodolfo for giving me access to a ML2/OVS env as we were able to spot why the /128 was not working. The agent was removing the LLA from the namespace and by keeping it, we\u0027re able to ensure that there\u0027s a route back from the metadata endpoint to the instance.\n\nAlso the patch ensures that the LLA is re-added to existing namespaces that do not have it when the agent gets restarted.","commit_id":"55557dbff2ef61169a6fdd52cb42cc003d729b6c"},{"author":{"_account_id":23804,"name":"Daniel Alvarez","email":"dalvarez@redhat.com","username":"dalvarez"},"change_message_id":"a1a0e421359646e785a07d7a6c4bd41a37859793","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"6a8f8d3d_84be9f9c","updated":"2023-09-14 05:29:17.000000000","message":"recheck","commit_id":"10d49d65cc87990697d42c6885287fdfa1f99e12"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"9e8090f731eaf76aeaf2c2d020c223e50e44cc0f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"5011f0c3_7d644544","updated":"2023-10-27 19:00:36.000000000","message":"Updated based on PTG discussion","commit_id":"1bd41edf5b617e61a47984d17feacf755caa7fbf"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"f94367d5151258204b15a91697b5ed5cd90f167f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"93941e4c_40510115","updated":"2023-09-14 22:21:59.000000000","message":"lgtm, thanks Daniel. Just have to wait for 2024.1 to open.","commit_id":"1bd41edf5b617e61a47984d17feacf755caa7fbf"},{"author":{"_account_id":23804,"name":"Daniel Alvarez","email":"dalvarez@redhat.com","username":"dalvarez"},"change_message_id":"38806c6ea0ff2842a7d6845bf4e3d3ee8552703c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"2eb3a12f_11943168","in_reply_to":"93941e4c_40510115","updated":"2023-09-15 15:39:16.000000000","message":"Thanks a lot Brian for the reviews!","commit_id":"1bd41edf5b617e61a47984d17feacf755caa7fbf"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"0726e95db276ec2cd1323717e9b1376227003cb8","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"36d218e6_568db363","updated":"2023-10-30 12:49:30.000000000","message":"Seems like some dead code is present?","commit_id":"bf80fced1b42940e594e9117db9b72bc34271306"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"7934e406f8f2b9e30a368752dde57467c91023c1","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"c16b9e20_4f34c95c","updated":"2023-10-27 21:23:02.000000000","message":"recheck unrelated test failure","commit_id":"bf80fced1b42940e594e9117db9b72bc34271306"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"82bc326b4a6783f52770db20695629cddd98eee1","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"2f2001a9_ff630e49","updated":"2023-11-06 21:54:09.000000000","message":"Let\u0027s see if I broke something...","commit_id":"94d3a9718466788a82d44f6ffc451ae7cefd9713"},{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"working for Ericsson, UTC+1 (+DST)"},"change_message_id":"8985617968efec7b2bbce036e330e162c4cc9789","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"a1925f3b_20ac7242","updated":"2023-11-21 09:44:17.000000000","message":"Something seems to be off compared to ps6.\n\n# ps6\n$ curl \u0027http://[fe80::a9fe:a9fe%25eth0]:80/\u0027\n1.0\n2007-01-19\n2007-03-01\n2007-08-29\n2007-10-10\n2007-12-15\n2008-02-01\n2008-09-01\n2009-04-04\n\n# ps10\n$ curl \u0027http://[fe80::a9fe:a9fe%25eth0]:80/\u0027\n\u003chtml\u003e\n \u003chead\u003e\n  \u003ctitle\u003e404 Not Found\u003c/title\u003e\n \u003c/head\u003e\n \u003cbody\u003e\n  \u003ch1\u003e404 Not Found\u003c/h1\u003e\n  The resource could not be found.\u003cbr /\u003e\u003cbr /\u003e\n\n\n\n \u003c/body\u003e\n\u003c/html\u003e","commit_id":"aee2930583790ae87636230d81022d8ee626a1c9"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"abb42bbaced1ea273388aff682b5bfb528bcba54","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"5ae67ad4_e429ba95","updated":"2023-11-15 00:46:20.000000000","message":"resolved merge conflict","commit_id":"aee2930583790ae87636230d81022d8ee626a1c9"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"4302d00f82b8fb3e91f88a6573cb2c136ad475e7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"e6f3a4c4_bdbbc451","in_reply_to":"a1925f3b_20ac7242","updated":"2023-11-21 15:32:45.000000000","message":"That means I broke it :(\n\nWill have to try and bisect and re-test.","commit_id":"aee2930583790ae87636230d81022d8ee626a1c9"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"51be68c60684d39626e21951ce1a01867e0849c2","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"e6d1d866_5f785fce","updated":"2023-12-01 15:35:26.000000000","message":"Marking important since there is a series of changes behind this one","commit_id":"5f626bdaf6ae504e7ea450b30f0997f6ff6c3897"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"a39d63355d50052beec2183a6f7dbeb1d4a71f04","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":12,"id":"49113a15_041b066e","updated":"2023-12-08 22:19:40.000000000","message":"Manual rebase due to https://review.opendev.org/c/openstack/neutron/+/896169","commit_id":"d9c8731af36d4eb53d9266733fec24659f2dc5a8"}],"neutron/agent/ovn/metadata/agent.py":[{"author":{"_account_id":23804,"name":"Daniel Alvarez","email":"dalvarez@redhat.com","username":"dalvarez"},"change_message_id":"95239d69bc5708351a438fe43a9abc7ec5ee8dee","unresolved":true,"context_lines":[{"line_number":485,"context_line":"            return []"},{"line_number":486,"context_line":"        mac_field_attrs \u003d port.mac[0].split()"},{"line_number":487,"context_line":"        ips \u003d mac_field_attrs[1:]"},{"line_number":488,"context_line":"        LOG.warning(\"XXXX 3 %s\", ips)"},{"line_number":489,"context_line":"        if not ips:"},{"line_number":490,"context_line":"            LOG.debug(\"Port %s IP addresses were not retrieved from the \""},{"line_number":491,"context_line":"                      \"Port_Binding MAC column %s\", port.uuid, mac_field_attrs)"}],"source_content_type":"text/x-python","patch_set":1,"id":"91f996fe_a657eef1","line":488,"range":{"start_line":488,"start_character":0,"end_line":488,"end_character":37},"updated":"2023-09-07 09:10:08.000000000","message":"to clean","commit_id":"6c507655131e6b0cf79ee81b2c2f26193ac12d37"},{"author":{"_account_id":23804,"name":"Daniel Alvarez","email":"dalvarez@redhat.com","username":"dalvarez"},"change_message_id":"8d125e7d83598660bcac8938800d7849f7ab2ba2","unresolved":false,"context_lines":[{"line_number":485,"context_line":"            return []"},{"line_number":486,"context_line":"        mac_field_attrs \u003d port.mac[0].split()"},{"line_number":487,"context_line":"        ips \u003d mac_field_attrs[1:]"},{"line_number":488,"context_line":"        LOG.warning(\"XXXX 3 %s\", ips)"},{"line_number":489,"context_line":"        if not ips:"},{"line_number":490,"context_line":"            LOG.debug(\"Port %s IP addresses were not retrieved from the \""},{"line_number":491,"context_line":"                      \"Port_Binding MAC column %s\", port.uuid, mac_field_attrs)"}],"source_content_type":"text/x-python","patch_set":1,"id":"cfc4a511_64efdeca","line":488,"range":{"start_line":488,"start_character":0,"end_line":488,"end_character":37},"in_reply_to":"91f996fe_a657eef1","updated":"2023-09-13 15:59:46.000000000","message":"Done","commit_id":"6c507655131e6b0cf79ee81b2c2f26193ac12d37"},{"author":{"_account_id":23804,"name":"Daniel Alvarez","email":"dalvarez@redhat.com","username":"dalvarez"},"change_message_id":"95239d69bc5708351a438fe43a9abc7ec5ee8dee","unresolved":true,"context_lines":[{"line_number":496,"context_line":"        # Prepopulate a dictionary where each metadata_port_cidr(string) maps"},{"line_number":497,"context_line":"        # to its netaddr.IPNetwork object. This is so we dont have to"},{"line_number":498,"context_line":"        # reconstruct IPNetwork objects repeatedly in the for loop"},{"line_number":499,"context_line":"        LOG.warning(\"XXX 1 %s\", metadata_port_cidrs)"},{"line_number":500,"context_line":"        LOG.warning(\"XXX 2 %s\", datapath_ports_ips)"},{"line_number":501,"context_line":""},{"line_number":502,"context_line":"        metadata_cidrs_to_network_objects \u003d {"},{"line_number":503,"context_line":"            metadata_port_cidr: netaddr.IPNetwork(metadata_port_cidr)"}],"source_content_type":"text/x-python","patch_set":1,"id":"1e376bb5_44e04723","line":500,"range":{"start_line":499,"start_character":0,"end_line":500,"end_character":51},"updated":"2023-09-07 09:10:08.000000000","message":"to clean","commit_id":"6c507655131e6b0cf79ee81b2c2f26193ac12d37"},{"author":{"_account_id":23804,"name":"Daniel Alvarez","email":"dalvarez@redhat.com","username":"dalvarez"},"change_message_id":"8d125e7d83598660bcac8938800d7849f7ab2ba2","unresolved":false,"context_lines":[{"line_number":496,"context_line":"        # Prepopulate a dictionary where each metadata_port_cidr(string) maps"},{"line_number":497,"context_line":"        # to its netaddr.IPNetwork object. This is so we dont have to"},{"line_number":498,"context_line":"        # reconstruct IPNetwork objects repeatedly in the for loop"},{"line_number":499,"context_line":"        LOG.warning(\"XXX 1 %s\", metadata_port_cidrs)"},{"line_number":500,"context_line":"        LOG.warning(\"XXX 2 %s\", datapath_ports_ips)"},{"line_number":501,"context_line":""},{"line_number":502,"context_line":"        metadata_cidrs_to_network_objects \u003d {"},{"line_number":503,"context_line":"            metadata_port_cidr: netaddr.IPNetwork(metadata_port_cidr)"}],"source_content_type":"text/x-python","patch_set":1,"id":"1a969de9_ca847613","line":500,"range":{"start_line":499,"start_character":0,"end_line":500,"end_character":51},"in_reply_to":"1e376bb5_44e04723","updated":"2023-09-13 15:59:46.000000000","message":"Done","commit_id":"6c507655131e6b0cf79ee81b2c2f26193ac12d37"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"ede7a2a3e055c3c0830311e3cf94ebe5c2084cb4","unresolved":true,"context_lines":[{"line_number":55,"context_line":"                                                               \u0027logical_port\u0027])"},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"OVN_METADATA_UUID_NAMESPACE \u003d uuid.UUID(\u0027d34bf9f6-da32-4871-9af8-15a4626b41ab\u0027)"},{"line_number":58,"context_line":"METADATA_V6_CIDR \u003d n_const.METADATA_V6_IP + \u0027/64\u0027"},{"line_number":59,"context_line":""},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"def _sync_lock(f):"}],"source_content_type":"text/x-python","patch_set":2,"id":"a967ec56_bd2820f1","line":58,"updated":"2023-09-07 14:11:41.000000000","message":"This should use the constant from neutron-lib, which has a /128 mask, which is what the \"other\" metadata agent uses.","commit_id":"fb154c6663b6b60fd417511c0100847d0202d44b"},{"author":{"_account_id":23804,"name":"Daniel Alvarez","email":"dalvarez@redhat.com","username":"dalvarez"},"change_message_id":"8d125e7d83598660bcac8938800d7849f7ab2ba2","unresolved":false,"context_lines":[{"line_number":55,"context_line":"                                                               \u0027logical_port\u0027])"},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"OVN_METADATA_UUID_NAMESPACE \u003d uuid.UUID(\u0027d34bf9f6-da32-4871-9af8-15a4626b41ab\u0027)"},{"line_number":58,"context_line":"METADATA_V6_CIDR \u003d n_const.METADATA_V6_IP + \u0027/64\u0027"},{"line_number":59,"context_line":""},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"def _sync_lock(f):"}],"source_content_type":"text/x-python","patch_set":2,"id":"9cdc1f5c_07cffbdb","line":58,"in_reply_to":"29f2d56f_dce0cc70","updated":"2023-09-13 15:59:46.000000000","message":"Done","commit_id":"fb154c6663b6b60fd417511c0100847d0202d44b"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"d351d8eb4e5dfa6617128378b5ba4d60b072a6e5","unresolved":true,"context_lines":[{"line_number":55,"context_line":"                                                               \u0027logical_port\u0027])"},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"OVN_METADATA_UUID_NAMESPACE \u003d uuid.UUID(\u0027d34bf9f6-da32-4871-9af8-15a4626b41ab\u0027)"},{"line_number":58,"context_line":"METADATA_V6_CIDR \u003d n_const.METADATA_V6_IP + \u0027/64\u0027"},{"line_number":59,"context_line":""},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"def _sync_lock(f):"}],"source_content_type":"text/x-python","patch_set":2,"id":"29f2d56f_dce0cc70","line":58,"in_reply_to":"3a6f5bba_5153c0e0","updated":"2023-09-11 13:06:25.000000000","message":"I\u0027ve been testing this patch, considering the main concern presented here that is the non-OVN metadata agent issue with the DAD failures with duplicated IPv6 addresses in HA environments. In the case of ML2/OVN, we don\u0027t have HA metadata servers but local metadata servers, that avoids the issue reported in [1].\n\nHowever, we\u0027ll have multiple metadata namespaces and ports, with the same IPv6 address, along the multiple compute nodes with VMs in the same network. However, as Daniel mentioned, these metadata ports are considered as \"type\u003dlocalport\". From the documentation [2]:\n\n\"\"\"\n       A localport logical switch port is a special kind of VIF logical\n       switch port. These ports are present in every chassis, not bound\n       to any particular one. Traffic to such a port will never be\n       forwarded through a tunnel, and traffic from such a port is\n       expected to be destined only to the same chassis, typically in\n       response to a request it received. OpenStack Neutron uses a\n       localport port to serve metadata to VMs. A metadata proxy process\n       is attached to this port on every host and all VMs within the\n       same network will reach it at the same IP/MAC address without any\n       traffic being sent over a tunnel. For further details, see the\n       OpenStack documentation for networking-ovn.\n\"\"\"\n\nThese ports are never bound to any chassis. For example, from an environment with two compute nodes with VMs in the same network and metadata namespaces: https://paste.opendev.org/show/bjRvmoDxmSNtbfU2RDX7/\n\nThe metadata HTTP request reaches the metadata IPv6 address using the LLA configured in the VM interface [3].\n\n[1]https://bugs.launchpad.net/neutron/+bug/1953165\n[2]https://man7.org/linux/man-pages/man7/ovn-architecture.7.html\n[3]https://paste.opendev.org/show/bW9ZGaZGjNgmNUZxlDp7/","commit_id":"fb154c6663b6b60fd417511c0100847d0202d44b"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"e0d39d58f6f9c1c41d90e14900f5dddcd679b1df","unresolved":true,"context_lines":[{"line_number":55,"context_line":"                                                               \u0027logical_port\u0027])"},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"OVN_METADATA_UUID_NAMESPACE \u003d uuid.UUID(\u0027d34bf9f6-da32-4871-9af8-15a4626b41ab\u0027)"},{"line_number":58,"context_line":"METADATA_V6_CIDR \u003d n_const.METADATA_V6_IP + \u0027/64\u0027"},{"line_number":59,"context_line":""},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"def _sync_lock(f):"}],"source_content_type":"text/x-python","patch_set":2,"id":"89ffbd2e_ea0c7c35","line":58,"in_reply_to":"4d02a844_366efa83","updated":"2023-09-07 22:14:53.000000000","message":"I think we have an ipv6-only job that tests such things? But guess any VM in ML2/OVS devstack setup should be able to wget/curl it.","commit_id":"fb154c6663b6b60fd417511c0100847d0202d44b"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"8f338f93645db2f64c8cd4666f0d4e354f18c40c","unresolved":true,"context_lines":[{"line_number":55,"context_line":"                                                               \u0027logical_port\u0027])"},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"OVN_METADATA_UUID_NAMESPACE \u003d uuid.UUID(\u0027d34bf9f6-da32-4871-9af8-15a4626b41ab\u0027)"},{"line_number":58,"context_line":"METADATA_V6_CIDR \u003d n_const.METADATA_V6_IP + \u0027/64\u0027"},{"line_number":59,"context_line":""},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"def _sync_lock(f):"}],"source_content_type":"text/x-python","patch_set":2,"id":"3a6f5bba_5153c0e0","line":58,"in_reply_to":"6f0ec73c_0e36d28c","updated":"2023-09-08 14:01:49.000000000","message":"I\u0027ll just throw it out there since I\u0027ve got a lot of downstream things to do today, but just wanted to make sure this bug in OVN isn\u0027t coming into play:\n\nhttps://bugs.launchpad.net/neutron/+bug/2031087\n\nIhar just fixed a bug affecting NAs in OVN. Don\u0027t know what would be different with OVN.","commit_id":"fb154c6663b6b60fd417511c0100847d0202d44b"},{"author":{"_account_id":23804,"name":"Daniel Alvarez","email":"dalvarez@redhat.com","username":"dalvarez"},"change_message_id":"1b6a116bda6b2768969cdbf1d2921a234549d3ab","unresolved":true,"context_lines":[{"line_number":55,"context_line":"                                                               \u0027logical_port\u0027])"},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"OVN_METADATA_UUID_NAMESPACE \u003d uuid.UUID(\u0027d34bf9f6-da32-4871-9af8-15a4626b41ab\u0027)"},{"line_number":58,"context_line":"METADATA_V6_CIDR \u003d n_const.METADATA_V6_IP + \u0027/64\u0027"},{"line_number":59,"context_line":""},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"def _sync_lock(f):"}],"source_content_type":"text/x-python","patch_set":2,"id":"6f0ec73c_0e36d28c","line":58,"in_reply_to":"89ffbd2e_ea0c7c35","updated":"2023-09-08 08:14:06.000000000","message":"yeah, with the current code it works and it passes the tempest job that I guess you\u0027re referring to. Also manually:\n\nubuntu@server1:~$ curl http://[fe80::a9fe:a9fe%25ens3]/\n1.0\n2007-01-19\n2007-03-01\n2007-08-29\n2007-10-10\n2007-12-15\n2008-02-01\n2008-09-01\n2009-04-04\n\n\nHowever, if I set the /128 mask, then the request comes from the LLA of the VM but will never be picked by the interface inside the namespace:\n\n$ sudo ip net e ovnmeta-582bdd7b-b3ce-4661-99f6-02bd7b487b0a tcpdump -vvnee  -i tap582bdd7b-b1 ip6\ntcpdump: listening on tap582bdd7b-b1, link-type EN10MB (Ethernet), snapshot length 262144 bytes\n^C08:13:11.969992 fa:16:3e:63:8d:c5 \u003e 33:33:00:00:00:02, ethertype IPv6 (0x86dd), length 70: (hlim 255, next-header ICMPv6 (58) payload length: 16) fe80::a9fe:a9fe \u003e ff02::2: [icmp6 sum ok] ICMP6, router solicitation, length 16\n          source link-address option (1), length 8 (1): fa:16:3e:63:8d:c5\n            0x0000:  fa16 3e63 8dc5\n08:13:13.700125 fa:16:3e:b7:16:8b \u003e fa:16:3e:63:8d:c5, ethertype IPv6 (0x86dd), length 94: (flowlabel 0x538a5, hlim 64, next-header TCP (6) payload length: 40) fe80::f816:3eff:feb7:168b.60478 \u003e fe80::a9fe:a9fe.80: Flags [S], cksum 0x8a37 (correct), seq 2185407737, win 64954, options [mss 1382,sackOK,TS val 1235729809 ecr 0,nop,wscale 7], length 0\n08:13:14.726775 fa:16:3e:b7:16:8b \u003e fa:16:3e:63:8d:c5, ethertype IPv6 (0x86dd), length 94: (flowlabel 0x7651e, hlim 64, next-header TCP (6) payload length: 40) fe80::f816:3eff:feb7:168b.60478 \u003e fe80::a9fe:a9fe.80: Flags [S], cksum 0x9d86 (incorrect -\u003e 0x8634), seq 2185407737, win 64954, options [mss 1382,sackOK,TS val 1235730836 ecr 0,nop,wscale 7], length 0\n08:13:16.742326 fa:16:3e:b7:16:8b \u003e fa:16:3e:63:8d:c5, ethertype IPv6 (0x86dd), length 94: (flowlabel 0x53113, hlim 64, next-header TCP (6) payload length: 40) fe80::f816:3eff:feb7:168b.60478 \u003e fe80::a9fe:a9fe.80: Flags [S], cksum 0x9d86 (incorrect -\u003e 0x7e54), seq 2185407737, win 64954, options [mss 1382,sackOK,TS val 1235732852 ecr 0,nop,wscale 7], length 0\n08:13:18.854680 fa:16:3e:b7:16:8b \u003e fa:16:3e:63:8d:c5, ethertype IPv6 (0x86dd), length 86: (hlim 255, next-header ICMPv6 (58) payload length: 32) fe80::f816:3eff:feb7:168b \u003e fe80::a9fe:a9fe: [icmp6 sum ok] ICMP6, neighbor solicitation, length 32, who has fe80::a9fe:a9fe\n          source link-address option (1), length 8 (1): fa:16:3e:b7:16:8b\n            0x0000:  fa16 3eb7 168b\n08:13:18.854716 fa:16:3e:63:8d:c5 \u003e fa:16:3e:b7:16:8b, ethertype IPv6 (0x86dd), length 78: (hlim 255, next-header ICMPv6 (58) payload length: 24) fe80::a9fe:a9fe \u003e fe80::f816:3eff:feb7:168b: [icmp6 sum ok] ICMP6, neighbor advertisement, length 24, tgt is fe80::a9fe:a9fe, Flags [solicited]","commit_id":"fb154c6663b6b60fd417511c0100847d0202d44b"},{"author":{"_account_id":23804,"name":"Daniel Alvarez","email":"dalvarez@redhat.com","username":"dalvarez"},"change_message_id":"a7b2ef0a1da88cd29338f8caabd2853d9c9ecb92","unresolved":true,"context_lines":[{"line_number":55,"context_line":"                                                               \u0027logical_port\u0027])"},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"OVN_METADATA_UUID_NAMESPACE \u003d uuid.UUID(\u0027d34bf9f6-da32-4871-9af8-15a4626b41ab\u0027)"},{"line_number":58,"context_line":"METADATA_V6_CIDR \u003d n_const.METADATA_V6_IP + \u0027/64\u0027"},{"line_number":59,"context_line":""},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"def _sync_lock(f):"}],"source_content_type":"text/x-python","patch_set":2,"id":"f28e1711_dc9c7bde","line":58,"in_reply_to":"a6169159_7fcf08cb","updated":"2023-09-07 15:33:50.000000000","message":"I meant: \"the initial patch had /64 and IIUC it was changed due to this DAD failure?\"","commit_id":"fb154c6663b6b60fd417511c0100847d0202d44b"},{"author":{"_account_id":23804,"name":"Daniel Alvarez","email":"dalvarez@redhat.com","username":"dalvarez"},"change_message_id":"5fdb29b7db2f8187960459e2a2ad62db60729c4d","unresolved":true,"context_lines":[{"line_number":55,"context_line":"                                                               \u0027logical_port\u0027])"},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"OVN_METADATA_UUID_NAMESPACE \u003d uuid.UUID(\u0027d34bf9f6-da32-4871-9af8-15a4626b41ab\u0027)"},{"line_number":58,"context_line":"METADATA_V6_CIDR \u003d n_const.METADATA_V6_IP + \u0027/64\u0027"},{"line_number":59,"context_line":""},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"def _sync_lock(f):"}],"source_content_type":"text/x-python","patch_set":2,"id":"a6169159_7fcf08cb","line":58,"in_reply_to":"a967ec56_bd2820f1","updated":"2023-09-07 15:26:50.000000000","message":"yeah, I mentioned this in the commit message.\nThe reason I\u0027m using /64 is to allow communication between the guest and the metadata port directly without having to allocate extra IP addresses for it and having to inject any static route.\n\nAlso, the initial patch had /128 and IIUC it was changed due to this DAD failure?\nhttps://github.com/openstack/neutron-lib/commit/a7b950cf8b5f322b7526b2b3eafb397b4f054c28\n\nFor this particular implementation there\u0027s no risk of DAD failures since the OVN metadata agent runs on every compute node and only one instance of it will do it.","commit_id":"fb154c6663b6b60fd417511c0100847d0202d44b"},{"author":{"_account_id":23804,"name":"Daniel Alvarez","email":"dalvarez@redhat.com","username":"dalvarez"},"change_message_id":"3775c96f9bbad28902809b6c83d748d81f0b9b20","unresolved":true,"context_lines":[{"line_number":55,"context_line":"                                                               \u0027logical_port\u0027])"},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"OVN_METADATA_UUID_NAMESPACE \u003d uuid.UUID(\u0027d34bf9f6-da32-4871-9af8-15a4626b41ab\u0027)"},{"line_number":58,"context_line":"METADATA_V6_CIDR \u003d n_const.METADATA_V6_IP + \u0027/64\u0027"},{"line_number":59,"context_line":""},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"def _sync_lock(f):"}],"source_content_type":"text/x-python","patch_set":2,"id":"4d02a844_366efa83","line":58,"in_reply_to":"bacffde9_59f45210","updated":"2023-09-07 20:37:07.000000000","message":"\" After looking into it (and how AWS did it), the /128 seemed to be the correct address/prefix.\n\nSo this won\u0027t work in this case?\"\n\n\nHmm, I can\u0027t see how... if we have /128, then when the VM sends the SYN packet to the port 80, haproxy won\u0027t reply right? Not sure how it\u0027s working on ML2/OVS though without a route?","commit_id":"fb154c6663b6b60fd417511c0100847d0202d44b"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"acf01506d159538622220e17eaf5a4be106b382b","unresolved":true,"context_lines":[{"line_number":55,"context_line":"                                                               \u0027logical_port\u0027])"},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"OVN_METADATA_UUID_NAMESPACE \u003d uuid.UUID(\u0027d34bf9f6-da32-4871-9af8-15a4626b41ab\u0027)"},{"line_number":58,"context_line":"METADATA_V6_CIDR \u003d n_const.METADATA_V6_IP + \u0027/64\u0027"},{"line_number":59,"context_line":""},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"def _sync_lock(f):"}],"source_content_type":"text/x-python","patch_set":2,"id":"bacffde9_59f45210","line":58,"in_reply_to":"f28e1711_dc9c7bde","updated":"2023-09-07 16:42:40.000000000","message":"Yes, it was changed for the DAD failure. My memory is a bit hazy but this is the comment I left:\n\nhttps://review.opendev.org/c/openstack/neutron/+/876566/10..13/neutron/agent/metadata/driver.py#b254\n\nBasically I think it had to do with the netaddr code and how it computed an address without a prefix given - we could not delete it properly when DAD failed. After looking into it (and how AWS did it), the /128 seemed to be the correct address/prefix.\n\nSo this won\u0027t work in this case?\n\nAnd so you don\u0027t go down that road, injecting an IPv6 static route into a VM isn\u0027t really possible, some of the later comments of mine in https://review.opendev.org/c/openstack/neutron/+/876903 help explain as that was an early attempt at doing metadata via a route. There is really only the concept of a default router.","commit_id":"fb154c6663b6b60fd417511c0100847d0202d44b"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"ea53a579aa840287353352124e8e04ec0a268e90","unresolved":true,"context_lines":[{"line_number":263,"context_line":"            resource_type\u003d\u0027metadata\u0027)"},{"line_number":264,"context_line":"        self._sb_idl \u003d None"},{"line_number":265,"context_line":"        self._post_fork_event \u003d threading.Event()"},{"line_number":266,"context_line":"        # We\u0027ll restart all haproxy instances upon start so that they honor"},{"line_number":267,"context_line":"        # any potential changes in their configuration."},{"line_number":268,"context_line":"        self.restarted_metadata_proxy_set \u003d set()"},{"line_number":269,"context_line":""},{"line_number":270,"context_line":"    @property"},{"line_number":271,"context_line":"    def sb_idl(self):"}],"source_content_type":"text/x-python","patch_set":3,"id":"145b755b_f80feedb","line":268,"range":{"start_line":266,"start_character":8,"end_line":268,"end_character":49},"updated":"2023-09-08 18:58:39.000000000","message":"wouldnt this happen when the `sync()` gets called L#338 in the `start`","commit_id":"5b2f37a1b851f635eab020e18981370ba130db0b"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"047660e2a23ed19eb1a42f04a7735caec6255fa9","unresolved":false,"context_lines":[{"line_number":263,"context_line":"            resource_type\u003d\u0027metadata\u0027)"},{"line_number":264,"context_line":"        self._sb_idl \u003d None"},{"line_number":265,"context_line":"        self._post_fork_event \u003d threading.Event()"},{"line_number":266,"context_line":"        # We\u0027ll restart all haproxy instances upon start so that they honor"},{"line_number":267,"context_line":"        # any potential changes in their configuration."},{"line_number":268,"context_line":"        self.restarted_metadata_proxy_set \u003d set()"},{"line_number":269,"context_line":""},{"line_number":270,"context_line":"    @property"},{"line_number":271,"context_line":"    def sb_idl(self):"}],"source_content_type":"text/x-python","patch_set":3,"id":"229969bc_83042cbe","line":268,"range":{"start_line":266,"start_character":8,"end_line":268,"end_character":49},"in_reply_to":"0ba96948_5d75e783","updated":"2023-09-15 20:06:21.000000000","message":"Spoke to Daniel offline. \nI was aware that when the agent is restarted the already running proxies are not restarted. But I was under impression that all containers(processes) are taken down during an \u0027upgrade\u0027. However, it looks like they are not. For this reason, I think this restart makes sense. We also discussed maybe more efficient restart where we would only restart if haproxy config file is different to what is already running. But that is a separate patch","commit_id":"5b2f37a1b851f635eab020e18981370ba130db0b"},{"author":{"_account_id":23804,"name":"Daniel Alvarez","email":"dalvarez@redhat.com","username":"dalvarez"},"change_message_id":"8e6935407233897e995b22685a5e26c11766374f","unresolved":true,"context_lines":[{"line_number":263,"context_line":"            resource_type\u003d\u0027metadata\u0027)"},{"line_number":264,"context_line":"        self._sb_idl \u003d None"},{"line_number":265,"context_line":"        self._post_fork_event \u003d threading.Event()"},{"line_number":266,"context_line":"        # We\u0027ll restart all haproxy instances upon start so that they honor"},{"line_number":267,"context_line":"        # any potential changes in their configuration."},{"line_number":268,"context_line":"        self.restarted_metadata_proxy_set \u003d set()"},{"line_number":269,"context_line":""},{"line_number":270,"context_line":"    @property"},{"line_number":271,"context_line":"    def sb_idl(self):"}],"source_content_type":"text/x-python","patch_set":3,"id":"8a8edec1_e13244aa","line":268,"range":{"start_line":266,"start_character":8,"end_line":268,"end_character":49},"in_reply_to":"145b755b_f80feedb","updated":"2023-09-13 15:41:59.000000000","message":"Not sure I get this...\nWithout this, then haproxy does not get restarted by ProcessManager, right?\nInstead, ProcessManager will see it is running and keep track of it without restarting and honoring the config changes.\nLet me know if it makes sense.","commit_id":"5b2f37a1b851f635eab020e18981370ba130db0b"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"5213e046045be433fa140fcb74a2b0ceefb70c9f","unresolved":true,"context_lines":[{"line_number":263,"context_line":"            resource_type\u003d\u0027metadata\u0027)"},{"line_number":264,"context_line":"        self._sb_idl \u003d None"},{"line_number":265,"context_line":"        self._post_fork_event \u003d threading.Event()"},{"line_number":266,"context_line":"        # We\u0027ll restart all haproxy instances upon start so that they honor"},{"line_number":267,"context_line":"        # any potential changes in their configuration."},{"line_number":268,"context_line":"        self.restarted_metadata_proxy_set \u003d set()"},{"line_number":269,"context_line":""},{"line_number":270,"context_line":"    @property"},{"line_number":271,"context_line":"    def sb_idl(self):"}],"source_content_type":"text/x-python","patch_set":3,"id":"e1ccbfa9_698bc3c0","line":268,"range":{"start_line":266,"start_character":8,"end_line":268,"end_character":49},"in_reply_to":"8a8edec1_e13244aa","updated":"2023-09-15 03:38:51.000000000","message":"We initialize the agent in metadata_agent.py and then we immediately call `agt.start()` method which will call the `sync` method. The `sync` method will `provision_datapath` for all the datapaths active on the chassis. In the `datapath _provision` we now add LLA(based on the metadata port MAC) into the namespace. Lastly by the end of the `provision_datapath` we call `spawn_monitored_metadata_proxy`. But now having this `restarted_metadata_proxy_set` we try to destroy the haproxy even though there is no haproxy running yet(if this was fresh start, just the first VM starting). I guess nothing would happen really because the process `disable` would just end up logging no process started [1]\n\nAnd then in the case when the agent is restarted but some haproxies are already running we might or might not restart haproxy depending if `netname` was already restarted. \nI guess I dont see the reason for the restart... Is it because the MAC of the metadata port could change and thus the LLA inside the namespace? I am just worried that we will do too many unnecessary stop/starts of haproxies. \n\n[1]https://opendev.org/openstack/neutron/src/commit/741f504c7badb3f6799977bd1e98ca5a9b7d1f53/neutron/agent/linux/external_process.py#L131","commit_id":"5b2f37a1b851f635eab020e18981370ba130db0b"},{"author":{"_account_id":23804,"name":"Daniel Alvarez","email":"dalvarez@redhat.com","username":"dalvarez"},"change_message_id":"38806c6ea0ff2842a7d6845bf4e3d3ee8552703c","unresolved":true,"context_lines":[{"line_number":263,"context_line":"            resource_type\u003d\u0027metadata\u0027)"},{"line_number":264,"context_line":"        self._sb_idl \u003d None"},{"line_number":265,"context_line":"        self._post_fork_event \u003d threading.Event()"},{"line_number":266,"context_line":"        # We\u0027ll restart all haproxy instances upon start so that they honor"},{"line_number":267,"context_line":"        # any potential changes in their configuration."},{"line_number":268,"context_line":"        self.restarted_metadata_proxy_set \u003d set()"},{"line_number":269,"context_line":""},{"line_number":270,"context_line":"    @property"},{"line_number":271,"context_line":"    def sb_idl(self):"}],"source_content_type":"text/x-python","patch_set":3,"id":"0ba96948_5d75e783","line":268,"range":{"start_line":266,"start_character":8,"end_line":268,"end_character":49},"in_reply_to":"939bdac1_048085c3","updated":"2023-09-15 15:39:16.000000000","message":"We could potentially compare the config files and trigger a restart if there\u0027s a delta but I\u0027d suggest doing that in a follow up patch (or in the refactor that Brian\u0027s proposing) since it affects also the ML2/OVS agents code as well.","commit_id":"5b2f37a1b851f635eab020e18981370ba130db0b"},{"author":{"_account_id":23804,"name":"Daniel Alvarez","email":"dalvarez@redhat.com","username":"dalvarez"},"change_message_id":"e1caf38d08aefdc6ece10a05686a25e040a801fb","unresolved":true,"context_lines":[{"line_number":263,"context_line":"            resource_type\u003d\u0027metadata\u0027)"},{"line_number":264,"context_line":"        self._sb_idl \u003d None"},{"line_number":265,"context_line":"        self._post_fork_event \u003d threading.Event()"},{"line_number":266,"context_line":"        # We\u0027ll restart all haproxy instances upon start so that they honor"},{"line_number":267,"context_line":"        # any potential changes in their configuration."},{"line_number":268,"context_line":"        self.restarted_metadata_proxy_set \u003d set()"},{"line_number":269,"context_line":""},{"line_number":270,"context_line":"    @property"},{"line_number":271,"context_line":"    def sb_idl(self):"}],"source_content_type":"text/x-python","patch_set":3,"id":"939bdac1_048085c3","line":268,"range":{"start_line":266,"start_character":8,"end_line":268,"end_character":49},"in_reply_to":"e1ccbfa9_698bc3c0","updated":"2023-09-15 15:34:01.000000000","message":"If we do not restart, the new config will not be honored right, then no IPv6 metadata for existing networks if there are workloads running on those nodes. The reason for the restart is that haproxy now will bind to the IPv6 address (we\u0027re changing the haproxy config file).\nAm I missing something?\n\nThe same logic is applied for ML2/OVS:\nhttps://review.opendev.org/c/openstack/neutron/+/715482/22/neutron/agent/dhcp/agent.py#121","commit_id":"5b2f37a1b851f635eab020e18981370ba130db0b"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"5662b36184d859b1936e43b6e10972b64f200a2f","unresolved":true,"context_lines":[{"line_number":641,"context_line":"        if cidrs_to_delete:"},{"line_number":642,"context_line":"            ip2.addr.delete_multiple(list(cidrs_to_delete))"},{"line_number":643,"context_line":""},{"line_number":644,"context_line":"        # NOTE(dalvarez): The only IPv6 address to add here is the LLA"},{"line_number":645,"context_line":"        # of the metadata service."},{"line_number":646,"context_line":"        ip_cidrs_to_add \u003d ["},{"line_number":647,"context_line":"            cidr"},{"line_number":648,"context_line":"            for cidr in cidrs_to_add"},{"line_number":649,"context_line":"            if (utils.get_ip_version(cidr) \u003d\u003d n_const.IP_VERSION_4 or"},{"line_number":650,"context_line":"                cidr \u003d\u003d METADATA_V6_CIDR)]"},{"line_number":651,"context_line":""},{"line_number":652,"context_line":"        if ip_cidrs_to_add:"},{"line_number":653,"context_line":"            ip2.addr.add_multiple(ip_cidrs_to_add)"}],"source_content_type":"text/x-python","patch_set":3,"id":"f371d84a_f4b2ac13","line":650,"range":{"start_line":644,"start_character":0,"end_line":650,"end_character":42},"updated":"2023-09-08 18:40:38.000000000","message":"Consider this:\nThis loop was here just to filter out all IPv6 addresses due IPv6 not being supported. It looks like that with the current implementation we dont really care about IPv6 addresses at all just the LLA.\nSo I propose that the `_get_port_ips` function only returns all IPv4 addresses. This loop will not be needed the size of all list/sets will get smaller. On #L487 we currently just take all IPs after the mac. So filter out IPv6 there. Here is an example of a SB port_binding record for your reference. \n```\n_uuid               : 69f4ff6e-cac0-414b-8a99-eb8cef84b2d0\nchassis             : 67b94fd2-0339-4ea4-ae21-39d54e9fc08f\ndatapath            : 257d314d-9ac8-4bd1-867a-37bb2d1ee976\nencap               : []\nexternal_ids        : {\"neutron:cidrs\"\u003d\"192.168.30.23/24 2001:db8:cafe:0:f816:3eff:fea0:bd90/64\", \"neutron:device_id\"\u003d\"ecfa13c2-95fe-492a-b662-853b2c0e419f\", \"neutron:device_owner\"\u003d\"compute:nova\", \"neutron:host_id\"\u003dvmone, \"neutron:network_name\"\u003dneutron-4d5934db-fa59-4e01-ac22-6fac60c9ad45, \"neutron:port_capabilities\"\u003d\"\", \"neutron:port_name\"\u003d\"\", \"neutron:project_id\"\u003da71a8414976040149f42378086fa5471, \"neutron:revision_number\"\u003d\"4\", \"neutron:security_group_ids\"\u003d\"df19b7ad-c28e-4c0f-aab4-f7a19f679116\", \"neutron:subnet_pool_addr_scope4\"\u003d\"\", \"neutron:subnet_pool_addr_scope6\"\u003d\"\", \"neutron:vnic_type\"\u003dnormal}\ngateway_chassis     : []\nha_chassis_group    : []\nlogical_port        : \"18bda35b-65c9-4aae-b557-63381c122dff\"\nmac                 : [\"fa:16:3e:a0:bd:90 192.168.30.23 2001:db8:cafe:0:f816:3eff:fea0:bd90\"]\nnat_addresses       : []\noptions             : {mcast_flood_reports\u003d\"true\", requested-chassis\u003dvmone}\nparent_port         : []\ntag                 : []\ntunnel_key          : 5\ntype                : \"\"\nvirtual_parent      : []\n```","commit_id":"5b2f37a1b851f635eab020e18981370ba130db0b"},{"author":{"_account_id":23804,"name":"Daniel Alvarez","email":"dalvarez@redhat.com","username":"dalvarez"},"change_message_id":"8e6935407233897e995b22685a5e26c11766374f","unresolved":false,"context_lines":[{"line_number":641,"context_line":"        if cidrs_to_delete:"},{"line_number":642,"context_line":"            ip2.addr.delete_multiple(list(cidrs_to_delete))"},{"line_number":643,"context_line":""},{"line_number":644,"context_line":"        # NOTE(dalvarez): The only IPv6 address to add here is the LLA"},{"line_number":645,"context_line":"        # of the metadata service."},{"line_number":646,"context_line":"        ip_cidrs_to_add \u003d ["},{"line_number":647,"context_line":"            cidr"},{"line_number":648,"context_line":"            for cidr in cidrs_to_add"},{"line_number":649,"context_line":"            if (utils.get_ip_version(cidr) \u003d\u003d n_const.IP_VERSION_4 or"},{"line_number":650,"context_line":"                cidr \u003d\u003d METADATA_V6_CIDR)]"},{"line_number":651,"context_line":""},{"line_number":652,"context_line":"        if ip_cidrs_to_add:"},{"line_number":653,"context_line":"            ip2.addr.add_multiple(ip_cidrs_to_add)"}],"source_content_type":"text/x-python","patch_set":3,"id":"cbe69975_93477be1","line":650,"range":{"start_line":644,"start_character":0,"end_line":650,"end_character":42},"in_reply_to":"f371d84a_f4b2ac13","updated":"2023-09-13 15:41:59.000000000","message":"Done","commit_id":"5b2f37a1b851f635eab020e18981370ba130db0b"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"5213e046045be433fa140fcb74a2b0ceefb70c9f","unresolved":true,"context_lines":[{"line_number":475,"context_line":"        iptables_mgr.ipv4[\u0027mangle\u0027].add_rule(\u0027POSTROUTING\u0027, rule, wrap\u003dFalse)"},{"line_number":476,"context_line":"        iptables_mgr.apply()"},{"line_number":477,"context_line":""},{"line_number":478,"context_line":"    def _get_port_ips(self, port):"},{"line_number":479,"context_line":"        # Retrieve IPs from the port mac column which is in form"},{"line_number":480,"context_line":"        # [\"\u003cport_mac\u003e \u003cip1\u003e \u003cip2\u003e ... \u003cipN\u003e\"]"},{"line_number":481,"context_line":"        if not port.mac:"}],"source_content_type":"text/x-python","patch_set":6,"id":"a05abdad_349fb1ff","line":478,"updated":"2023-09-15 03:38:51.000000000","message":"small nit. Rename it to `_get_port_ip4_ips` now that ipv6 are filtered out","commit_id":"1bd41edf5b617e61a47984d17feacf755caa7fbf"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"9e8090f731eaf76aeaf2c2d020c223e50e44cc0f","unresolved":false,"context_lines":[{"line_number":475,"context_line":"        iptables_mgr.ipv4[\u0027mangle\u0027].add_rule(\u0027POSTROUTING\u0027, rule, wrap\u003dFalse)"},{"line_number":476,"context_line":"        iptables_mgr.apply()"},{"line_number":477,"context_line":""},{"line_number":478,"context_line":"    def _get_port_ips(self, port):"},{"line_number":479,"context_line":"        # Retrieve IPs from the port mac column which is in form"},{"line_number":480,"context_line":"        # [\"\u003cport_mac\u003e \u003cip1\u003e \u003cip2\u003e ... \u003cipN\u003e\"]"},{"line_number":481,"context_line":"        if not port.mac:"}],"source_content_type":"text/x-python","patch_set":6,"id":"9429ea1e_80211cde","line":478,"in_reply_to":"9cc872aa_88b5a70b","updated":"2023-10-27 19:00:36.000000000","message":"Done","commit_id":"1bd41edf5b617e61a47984d17feacf755caa7fbf"},{"author":{"_account_id":23804,"name":"Daniel Alvarez","email":"dalvarez@redhat.com","username":"dalvarez"},"change_message_id":"e1caf38d08aefdc6ece10a05686a25e040a801fb","unresolved":true,"context_lines":[{"line_number":475,"context_line":"        iptables_mgr.ipv4[\u0027mangle\u0027].add_rule(\u0027POSTROUTING\u0027, rule, wrap\u003dFalse)"},{"line_number":476,"context_line":"        iptables_mgr.apply()"},{"line_number":477,"context_line":""},{"line_number":478,"context_line":"    def _get_port_ips(self, port):"},{"line_number":479,"context_line":"        # Retrieve IPs from the port mac column which is in form"},{"line_number":480,"context_line":"        # [\"\u003cport_mac\u003e \u003cip1\u003e \u003cip2\u003e ... \u003cipN\u003e\"]"},{"line_number":481,"context_line":"        if not port.mac:"}],"source_content_type":"text/x-python","patch_set":6,"id":"9cc872aa_88b5a70b","line":478,"in_reply_to":"a05abdad_349fb1ff","updated":"2023-09-15 15:34:01.000000000","message":"ack, will change in next respin","commit_id":"1bd41edf5b617e61a47984d17feacf755caa7fbf"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"6cd3a354d206f3b634fe135f9ad70f7f67785c01","unresolved":true,"context_lines":[{"line_number":516,"context_line":""},{"line_number":517,"context_line":"        cidrs_to_add \u003d active_subnets_cidrs - current_namespace_cidrs"},{"line_number":518,"context_line":""},{"line_number":519,"context_line":"        if n_const.METADATA_CIDR not in current_namespace_cidrs:"},{"line_number":520,"context_line":"            cidrs_to_add.add(n_const.METADATA_CIDR)"},{"line_number":521,"context_line":"        else:"},{"line_number":522,"context_line":"            active_subnets_cidrs.add(n_const.METADATA_CIDR)"},{"line_number":523,"context_line":""},{"line_number":524,"context_line":"        if n_const.METADATA_V6_CIDR not in current_namespace_cidrs:"},{"line_number":525,"context_line":"            cidrs_to_add.add(n_const.METADATA_V6_CIDR)"},{"line_number":526,"context_line":"        else:"},{"line_number":527,"context_line":"            active_subnets_cidrs.add(n_const.METADATA_V6_CIDR)"},{"line_number":528,"context_line":""},{"line_number":529,"context_line":"        # Make sure that the LLA of the interface is present"},{"line_number":530,"context_line":"        if lla not in current_namespace_cidrs:"},{"line_number":531,"context_line":"            cidrs_to_add.add(lla)"},{"line_number":532,"context_line":"        else:"},{"line_number":533,"context_line":"            active_subnets_cidrs.add(lla)"},{"line_number":534,"context_line":""},{"line_number":535,"context_line":"        cidrs_to_delete \u003d current_namespace_cidrs - active_subnets_cidrs"},{"line_number":536,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"a9ea9b60_c2791e81","line":533,"range":{"start_line":519,"start_character":0,"end_line":533,"end_character":41},"updated":"2023-09-20 19:34:16.000000000","message":"```\nfor addr in (n_const.METADATA_CIDR, n_const.METADATA_V6_CIDR, lla):\n    if addr not in current_namespace_cidrs:\n        cidrs_to_add.add(addr)\n    else:\n        active_subnets_cidrs.add(addr)\n```","commit_id":"1bd41edf5b617e61a47984d17feacf755caa7fbf"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"9e8090f731eaf76aeaf2c2d020c223e50e44cc0f","unresolved":false,"context_lines":[{"line_number":516,"context_line":""},{"line_number":517,"context_line":"        cidrs_to_add \u003d active_subnets_cidrs - current_namespace_cidrs"},{"line_number":518,"context_line":""},{"line_number":519,"context_line":"        if n_const.METADATA_CIDR not in current_namespace_cidrs:"},{"line_number":520,"context_line":"            cidrs_to_add.add(n_const.METADATA_CIDR)"},{"line_number":521,"context_line":"        else:"},{"line_number":522,"context_line":"            active_subnets_cidrs.add(n_const.METADATA_CIDR)"},{"line_number":523,"context_line":""},{"line_number":524,"context_line":"        if n_const.METADATA_V6_CIDR not in current_namespace_cidrs:"},{"line_number":525,"context_line":"            cidrs_to_add.add(n_const.METADATA_V6_CIDR)"},{"line_number":526,"context_line":"        else:"},{"line_number":527,"context_line":"            active_subnets_cidrs.add(n_const.METADATA_V6_CIDR)"},{"line_number":528,"context_line":""},{"line_number":529,"context_line":"        # Make sure that the LLA of the interface is present"},{"line_number":530,"context_line":"        if lla not in current_namespace_cidrs:"},{"line_number":531,"context_line":"            cidrs_to_add.add(lla)"},{"line_number":532,"context_line":"        else:"},{"line_number":533,"context_line":"            active_subnets_cidrs.add(lla)"},{"line_number":534,"context_line":""},{"line_number":535,"context_line":"        cidrs_to_delete \u003d current_namespace_cidrs - active_subnets_cidrs"},{"line_number":536,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"c061258d_7bcef468","line":533,"range":{"start_line":519,"start_character":0,"end_line":533,"end_character":41},"in_reply_to":"17ea5c56_4b4dc914","updated":"2023-10-27 19:00:36.000000000","message":"I\u0027ll change this as Kuba suggested.","commit_id":"1bd41edf5b617e61a47984d17feacf755caa7fbf"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"848743702519bad7885f99204ac26c381c1a9220","unresolved":true,"context_lines":[{"line_number":516,"context_line":""},{"line_number":517,"context_line":"        cidrs_to_add \u003d active_subnets_cidrs - current_namespace_cidrs"},{"line_number":518,"context_line":""},{"line_number":519,"context_line":"        if n_const.METADATA_CIDR not in current_namespace_cidrs:"},{"line_number":520,"context_line":"            cidrs_to_add.add(n_const.METADATA_CIDR)"},{"line_number":521,"context_line":"        else:"},{"line_number":522,"context_line":"            active_subnets_cidrs.add(n_const.METADATA_CIDR)"},{"line_number":523,"context_line":""},{"line_number":524,"context_line":"        if n_const.METADATA_V6_CIDR not in current_namespace_cidrs:"},{"line_number":525,"context_line":"            cidrs_to_add.add(n_const.METADATA_V6_CIDR)"},{"line_number":526,"context_line":"        else:"},{"line_number":527,"context_line":"            active_subnets_cidrs.add(n_const.METADATA_V6_CIDR)"},{"line_number":528,"context_line":""},{"line_number":529,"context_line":"        # Make sure that the LLA of the interface is present"},{"line_number":530,"context_line":"        if lla not in current_namespace_cidrs:"},{"line_number":531,"context_line":"            cidrs_to_add.add(lla)"},{"line_number":532,"context_line":"        else:"},{"line_number":533,"context_line":"            active_subnets_cidrs.add(lla)"},{"line_number":534,"context_line":""},{"line_number":535,"context_line":"        cidrs_to_delete \u003d current_namespace_cidrs - active_subnets_cidrs"},{"line_number":536,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"17ea5c56_4b4dc914","line":533,"range":{"start_line":519,"start_character":0,"end_line":533,"end_character":41},"in_reply_to":"a9ea9b60_c2791e81","updated":"2023-09-20 21:05:05.000000000","message":"I thought the same thing when I saw it, but then why not just replace the whole mess with set arithmetic :-P\n\n    def _process_cidrs(self, current_namespace_cidrs,\n                       datapath_ports_ips, metadata_port_subnet_cidrs, lla):\n        active_subnets_cidrs \u003d self._active_subnets_cidrs(\n            datapath_ports_ips, metadata_port_subnet_cidrs)\n\n        required_cidrs \u003d set((n_const.METADATA_CIDR, n_const.METADATA_V6_CIDR,\n                              lla))\n        required_cidrs.update(active_subnets_cidrs)\n\n        cidrs_to_add \u003d required_cidrs - current_namespace_cidrs\n\n        cidrs_to_delete \u003d current_namespace_cidrs - required_cidrs\n\n        return cidrs_to_add, cidrs_to_delete","commit_id":"1bd41edf5b617e61a47984d17feacf755caa7fbf"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"f94367d5151258204b15a91697b5ed5cd90f167f","unresolved":true,"context_lines":[{"line_number":560,"context_line":"                      \"MAC address configured, tearing the namespace \""},{"line_number":561,"context_line":"                      \"down if needed\", net_name)"},{"line_number":562,"context_line":"            self.teardown_datapath(net_name)"},{"line_number":563,"context_line":"            return"},{"line_number":564,"context_line":""},{"line_number":565,"context_line":"        # First entry of the mac field must be the MAC address."},{"line_number":566,"context_line":"        match \u003d MAC_PATTERN.match(metadata_port.mac[0].split(\u0027 \u0027)[0])"}],"source_content_type":"text/x-python","patch_set":6,"id":"d6158416_de93df8b","line":563,"updated":"2023-09-14 22:21:59.000000000","message":"There\u0027s another patch in this area as well, eventually we\u0027ll give it some attention:\n\nhttps://review.opendev.org/c/openstack/neutron/+/881487\n\nJust an FYI.","commit_id":"1bd41edf5b617e61a47984d17feacf755caa7fbf"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"5213e046045be433fa140fcb74a2b0ceefb70c9f","unresolved":true,"context_lines":[{"line_number":640,"context_line":"        cidrs_to_add, cidrs_to_delete \u003d self._process_cidrs("},{"line_number":641,"context_line":"            {dev[\u0027cidr\u0027] for dev in ip2.addr.list()},"},{"line_number":642,"context_line":"            datapath_ports_ips,"},{"line_number":643,"context_line":"            metadata_port_info.ip_addresses,"},{"line_number":644,"context_line":"            ip_lib.get_ipv6_lladdr(metadata_port_info.mac)"},{"line_number":645,"context_line":"        )"},{"line_number":646,"context_line":""},{"line_number":647,"context_line":"        # Delete any non active addresses from the network namespace"}],"source_content_type":"text/x-python","patch_set":6,"id":"adf972ad_4bbb90b9","line":644,"range":{"start_line":643,"start_character":0,"end_line":644,"end_character":57},"updated":"2023-09-15 03:38:51.000000000","message":"nit. You can just pass the metada_port_into object and then access `.ip_address` and `.mac` inside the function. It will reduce number of parameters this function takes","commit_id":"1bd41edf5b617e61a47984d17feacf755caa7fbf"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"9e8090f731eaf76aeaf2c2d020c223e50e44cc0f","unresolved":false,"context_lines":[{"line_number":640,"context_line":"        cidrs_to_add, cidrs_to_delete \u003d self._process_cidrs("},{"line_number":641,"context_line":"            {dev[\u0027cidr\u0027] for dev in ip2.addr.list()},"},{"line_number":642,"context_line":"            datapath_ports_ips,"},{"line_number":643,"context_line":"            metadata_port_info.ip_addresses,"},{"line_number":644,"context_line":"            ip_lib.get_ipv6_lladdr(metadata_port_info.mac)"},{"line_number":645,"context_line":"        )"},{"line_number":646,"context_line":""},{"line_number":647,"context_line":"        # Delete any non active addresses from the network namespace"}],"source_content_type":"text/x-python","patch_set":6,"id":"a618a698_ab0fda30","line":644,"range":{"start_line":643,"start_character":0,"end_line":644,"end_character":57},"in_reply_to":"2dabb455_1e19d713","updated":"2023-10-27 19:00:36.000000000","message":"Let\u0027s just leave this as-is.","commit_id":"1bd41edf5b617e61a47984d17feacf755caa7fbf"},{"author":{"_account_id":23804,"name":"Daniel Alvarez","email":"dalvarez@redhat.com","username":"dalvarez"},"change_message_id":"e1caf38d08aefdc6ece10a05686a25e040a801fb","unresolved":true,"context_lines":[{"line_number":640,"context_line":"        cidrs_to_add, cidrs_to_delete \u003d self._process_cidrs("},{"line_number":641,"context_line":"            {dev[\u0027cidr\u0027] for dev in ip2.addr.list()},"},{"line_number":642,"context_line":"            datapath_ports_ips,"},{"line_number":643,"context_line":"            metadata_port_info.ip_addresses,"},{"line_number":644,"context_line":"            ip_lib.get_ipv6_lladdr(metadata_port_info.mac)"},{"line_number":645,"context_line":"        )"},{"line_number":646,"context_line":""},{"line_number":647,"context_line":"        # Delete any non active addresses from the network namespace"}],"source_content_type":"text/x-python","patch_set":6,"id":"2dabb455_1e19d713","line":644,"range":{"start_line":643,"start_character":0,"end_line":644,"end_character":57},"in_reply_to":"adf972ad_4bbb90b9","updated":"2023-09-15 15:34:01.000000000","message":"I\u0027d rather pass the LLA and not the MAC because we are passing only IPs and _process_cidrs is dealing just with IPs but it won\u0027t hurt to do what you say. Unless other reviewers agree with this change I\u0027ll keep it like this if that\u0027s ok.","commit_id":"1bd41edf5b617e61a47984d17feacf755caa7fbf"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"0726e95db276ec2cd1323717e9b1376227003cb8","unresolved":true,"context_lines":[{"line_number":264,"context_line":"        self._post_fork_event \u003d threading.Event()"},{"line_number":265,"context_line":"        # We\u0027ll restart all haproxy instances upon start so that they honor"},{"line_number":266,"context_line":"        # any potential changes in their configuration."},{"line_number":267,"context_line":"        self.restarted_metadata_proxy_set \u003d set()"},{"line_number":268,"context_line":""},{"line_number":269,"context_line":"    @property"},{"line_number":270,"context_line":"    def sb_idl(self):"}],"source_content_type":"text/x-python","patch_set":7,"id":"beb3b52c_7fd1c798","line":267,"updated":"2023-10-30 12:49:30.000000000","message":"Is this used anywhere?","commit_id":"bf80fced1b42940e594e9117db9b72bc34271306"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"bf934b6d2cc6124f36f50533615e96de9fe4c97b","unresolved":false,"context_lines":[{"line_number":264,"context_line":"        self._post_fork_event \u003d threading.Event()"},{"line_number":265,"context_line":"        # We\u0027ll restart all haproxy instances upon start so that they honor"},{"line_number":266,"context_line":"        # any potential changes in their configuration."},{"line_number":267,"context_line":"        self.restarted_metadata_proxy_set \u003d set()"},{"line_number":268,"context_line":""},{"line_number":269,"context_line":"    @property"},{"line_number":270,"context_line":"    def sb_idl(self):"}],"source_content_type":"text/x-python","patch_set":7,"id":"569a45ae_ef4594fb","line":267,"in_reply_to":"05f9010f_29beee35","updated":"2023-10-30 17:23:07.000000000","message":"Done","commit_id":"bf80fced1b42940e594e9117db9b72bc34271306"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"feaa0a4eda85fa0057cd77b67e2aae57e59182d1","unresolved":true,"context_lines":[{"line_number":264,"context_line":"        self._post_fork_event \u003d threading.Event()"},{"line_number":265,"context_line":"        # We\u0027ll restart all haproxy instances upon start so that they honor"},{"line_number":266,"context_line":"        # any potential changes in their configuration."},{"line_number":267,"context_line":"        self.restarted_metadata_proxy_set \u003d set()"},{"line_number":268,"context_line":""},{"line_number":269,"context_line":"    @property"},{"line_number":270,"context_line":"    def sb_idl(self):"}],"source_content_type":"text/x-python","patch_set":7,"id":"05f9010f_29beee35","line":267,"in_reply_to":"beb3b52c_7fd1c798","updated":"2023-10-30 12:50:17.000000000","message":"Oh, I see. It\u0027s to avoid calling destroy, please ignore :)","commit_id":"bf80fced1b42940e594e9117db9b72bc34271306"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"5f9ee258b7cf71e7013246c32b8ad5a39520d748","unresolved":true,"context_lines":[{"line_number":544,"context_line":"        datapath_uuid \u003d str(datapath.uuid)"},{"line_number":545,"context_line":""},{"line_number":546,"context_line":"        metadata_port \u003d self.sb_idl.get_metadata_port_network(datapath_uuid)"},{"line_number":547,"context_line":"        # If there\u0027s no metadata port or it doesn\u0027t have a MAC address, then"},{"line_number":548,"context_line":"        # tear the namespace down if needed."},{"line_number":549,"context_line":"        if not (metadata_port and metadata_port.mac):"},{"line_number":550,"context_line":"            LOG.debug(\"There is no metadata port for network %s or it has no \""},{"line_number":551,"context_line":"                      \"MAC address configured, tearing the namespace \""}],"source_content_type":"text/x-python","patch_set":7,"id":"8741fe6b_b9314b33","line":548,"range":{"start_line":547,"start_character":0,"end_line":548,"end_character":44},"updated":"2023-10-30 20:13:57.000000000","message":"I would remove this comment. By removing the sentence which explained the \"why?\" from the comment, this comment is not needed. It now just describes what the code will do which is self explanatory.","commit_id":"bf80fced1b42940e594e9117db9b72bc34271306"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"d9cac8fbc16883381d09d16c69a43485a86a7e6d","unresolved":true,"context_lines":[{"line_number":544,"context_line":"        datapath_uuid \u003d str(datapath.uuid)"},{"line_number":545,"context_line":""},{"line_number":546,"context_line":"        metadata_port \u003d self.sb_idl.get_metadata_port_network(datapath_uuid)"},{"line_number":547,"context_line":"        # If there\u0027s no metadata port or it doesn\u0027t have a MAC address, then"},{"line_number":548,"context_line":"        # tear the namespace down if needed."},{"line_number":549,"context_line":"        if not (metadata_port and metadata_port.mac):"},{"line_number":550,"context_line":"            LOG.debug(\"There is no metadata port for network %s or it has no \""},{"line_number":551,"context_line":"                      \"MAC address configured, tearing the namespace \""}],"source_content_type":"text/x-python","patch_set":7,"id":"16b980e3_e4f15daf","line":548,"range":{"start_line":547,"start_character":0,"end_line":548,"end_character":44},"in_reply_to":"8741fe6b_b9314b33","updated":"2023-10-30 20:44:18.000000000","message":"If we have to respin I can remove it since it\u0027 just a nit IMO.","commit_id":"bf80fced1b42940e594e9117db9b72bc34271306"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"5f9ee258b7cf71e7013246c32b8ad5a39520d748","unresolved":true,"context_lines":[{"line_number":632,"context_line":"            {dev[\u0027cidr\u0027] for dev in ip2.addr.list()},"},{"line_number":633,"context_line":"            datapath_ports_ips,"},{"line_number":634,"context_line":"            metadata_port_info.ip_addresses,"},{"line_number":635,"context_line":"            ip_lib.get_ipv6_lladdr(metadata_port_info.mac)"},{"line_number":636,"context_line":"        )"},{"line_number":637,"context_line":""},{"line_number":638,"context_line":"        # Delete any non active addresses from the network namespace"}],"source_content_type":"text/x-python","patch_set":7,"id":"8ca528ef_46b99d7c","line":635,"range":{"start_line":635,"start_character":12,"end_line":635,"end_character":58},"updated":"2023-10-30 20:13:57.000000000","message":"ni. you could just have the function accept `metadata_port_info` object and the funtion itself can retrieve `ip_address` and `lla`","commit_id":"bf80fced1b42940e594e9117db9b72bc34271306"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"d9cac8fbc16883381d09d16c69a43485a86a7e6d","unresolved":false,"context_lines":[{"line_number":632,"context_line":"            {dev[\u0027cidr\u0027] for dev in ip2.addr.list()},"},{"line_number":633,"context_line":"            datapath_ports_ips,"},{"line_number":634,"context_line":"            metadata_port_info.ip_addresses,"},{"line_number":635,"context_line":"            ip_lib.get_ipv6_lladdr(metadata_port_info.mac)"},{"line_number":636,"context_line":"        )"},{"line_number":637,"context_line":""},{"line_number":638,"context_line":"        # Delete any non active addresses from the network namespace"}],"source_content_type":"text/x-python","patch_set":7,"id":"b7421367_5ad31923","line":635,"range":{"start_line":635,"start_character":12,"end_line":635,"end_character":58},"in_reply_to":"8ca528ef_46b99d7c","updated":"2023-10-30 20:44:18.000000000","message":"Response from Daniel on previous PS:\n\nI\u0027d rather pass the LLA and not the MAC because we are passing only IPs and _process_cidrs is dealing just with IPs but it won\u0027t hurt to do what you say. Unless other reviewers agree with this change I\u0027ll keep it like this if that\u0027s ok.\n\nSo I would just leave this as-is.","commit_id":"bf80fced1b42940e594e9117db9b72bc34271306"}],"neutron/agent/ovn/metadata/driver.py":[{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"ede7a2a3e055c3c0830311e3cf94ebe5c2084cb4","unresolved":true,"context_lines":[{"line_number":197,"context_line":"    def spawn_monitored_metadata_proxy(cls, monitor, ns_name, port, conf,"},{"line_number":198,"context_line":"                                       bind_address\u003d\"0.0.0.0\", network_id\u003dNone,"},{"line_number":199,"context_line":"                                       router_id\u003dNone, bind_address_v6\u003dNone,"},{"line_number":200,"context_line":"                                       bind_interface\u003dNone):"},{"line_number":201,"context_line":"        uuid \u003d network_id or router_id"},{"line_number":202,"context_line":"        callback \u003d cls._get_metadata_proxy_callback("},{"line_number":203,"context_line":"            bind_address, port, conf, network_id\u003dnetwork_id,"}],"source_content_type":"text/x-python","patch_set":2,"id":"cfb1a2af_7780357a","line":200,"updated":"2023-09-07 14:11:41.000000000","message":"There was a recent bug, 1953165, that required a change in this code in the \"other\" metadata driver to detect and ignore DAD failures. Since this is also using haproxy I would assume we need it here as well.\n\nOf course we\u0027ve now duplicated most of this code between the two drivers, will have to see if we can combine things eventually.","commit_id":"fb154c6663b6b60fd417511c0100847d0202d44b"},{"author":{"_account_id":23804,"name":"Daniel Alvarez","email":"dalvarez@redhat.com","username":"dalvarez"},"change_message_id":"0a79d4508e605fec9fc28e4787ad2a0d71dc9801","unresolved":false,"context_lines":[{"line_number":197,"context_line":"    def spawn_monitored_metadata_proxy(cls, monitor, ns_name, port, conf,"},{"line_number":198,"context_line":"                                       bind_address\u003d\"0.0.0.0\", network_id\u003dNone,"},{"line_number":199,"context_line":"                                       router_id\u003dNone, bind_address_v6\u003dNone,"},{"line_number":200,"context_line":"                                       bind_interface\u003dNone):"},{"line_number":201,"context_line":"        uuid \u003d network_id or router_id"},{"line_number":202,"context_line":"        callback \u003d cls._get_metadata_proxy_callback("},{"line_number":203,"context_line":"            bind_address, port, conf, network_id\u003dnetwork_id,"}],"source_content_type":"text/x-python","patch_set":2,"id":"eccc6902_30aacffc","line":200,"in_reply_to":"3cd2ef05_86719697","updated":"2023-09-13 16:00:04.000000000","message":"Done","commit_id":"fb154c6663b6b60fd417511c0100847d0202d44b"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"acf01506d159538622220e17eaf5a4be106b382b","unresolved":true,"context_lines":[{"line_number":197,"context_line":"    def spawn_monitored_metadata_proxy(cls, monitor, ns_name, port, conf,"},{"line_number":198,"context_line":"                                       bind_address\u003d\"0.0.0.0\", network_id\u003dNone,"},{"line_number":199,"context_line":"                                       router_id\u003dNone, bind_address_v6\u003dNone,"},{"line_number":200,"context_line":"                                       bind_interface\u003dNone):"},{"line_number":201,"context_line":"        uuid \u003d network_id or router_id"},{"line_number":202,"context_line":"        callback \u003d cls._get_metadata_proxy_callback("},{"line_number":203,"context_line":"            bind_address, port, conf, network_id\u003dnetwork_id,"}],"source_content_type":"text/x-python","patch_set":2,"id":"f1d75ad8_20616622","line":200,"in_reply_to":"c21e3273_eebe9bb8","updated":"2023-09-07 16:42:40.000000000","message":"Ack.\n\nThe one thing that has to be done in this case though is to wait for the IPv6 address to leave the \"tentative\" state when DAD completes, else haproxy will fail to bind() the address. There might be a workaround by adding a \"transparent\" flag to haproxy, https://docs.haproxy.org/2.4/configuration.html#5.1-transparent but we can maybe do that separately.\n\nFor now it\u0027s probably safe to just copy the code since DAD should never fail anyways as you mentioned.","commit_id":"fb154c6663b6b60fd417511c0100847d0202d44b"},{"author":{"_account_id":23804,"name":"Daniel Alvarez","email":"dalvarez@redhat.com","username":"dalvarez"},"change_message_id":"5fdb29b7db2f8187960459e2a2ad62db60729c4d","unresolved":true,"context_lines":[{"line_number":197,"context_line":"    def spawn_monitored_metadata_proxy(cls, monitor, ns_name, port, conf,"},{"line_number":198,"context_line":"                                       bind_address\u003d\"0.0.0.0\", network_id\u003dNone,"},{"line_number":199,"context_line":"                                       router_id\u003dNone, bind_address_v6\u003dNone,"},{"line_number":200,"context_line":"                                       bind_interface\u003dNone):"},{"line_number":201,"context_line":"        uuid \u003d network_id or router_id"},{"line_number":202,"context_line":"        callback \u003d cls._get_metadata_proxy_callback("},{"line_number":203,"context_line":"            bind_address, port, conf, network_id\u003dnetwork_id,"}],"source_content_type":"text/x-python","patch_set":2,"id":"c21e3273_eebe9bb8","line":200,"in_reply_to":"cfb1a2af_7780357a","updated":"2023-09-07 15:26:50.000000000","message":"Please see my comment above about the DAD failures which I don\u0027t think this implementation would suffer from","commit_id":"fb154c6663b6b60fd417511c0100847d0202d44b"},{"author":{"_account_id":23804,"name":"Daniel Alvarez","email":"dalvarez@redhat.com","username":"dalvarez"},"change_message_id":"3775c96f9bbad28902809b6c83d748d81f0b9b20","unresolved":true,"context_lines":[{"line_number":197,"context_line":"    def spawn_monitored_metadata_proxy(cls, monitor, ns_name, port, conf,"},{"line_number":198,"context_line":"                                       bind_address\u003d\"0.0.0.0\", network_id\u003dNone,"},{"line_number":199,"context_line":"                                       router_id\u003dNone, bind_address_v6\u003dNone,"},{"line_number":200,"context_line":"                                       bind_interface\u003dNone):"},{"line_number":201,"context_line":"        uuid \u003d network_id or router_id"},{"line_number":202,"context_line":"        callback \u003d cls._get_metadata_proxy_callback("},{"line_number":203,"context_line":"            bind_address, port, conf, network_id\u003dnetwork_id,"}],"source_content_type":"text/x-python","patch_set":2,"id":"3cd2ef05_86719697","line":200,"in_reply_to":"f1d75ad8_20616622","updated":"2023-09-07 20:37:07.000000000","message":"Good point, I\u0027m sending a new PS with that code in a min. Thanks Brian!","commit_id":"fb154c6663b6b60fd417511c0100847d0202d44b"},{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"working for Ericsson, UTC+1 (+DST)"},"change_message_id":"c2f29a88cbd282a509d3f40ca0f6e50036466563","unresolved":true,"context_lines":[{"line_number":35,"context_line":"PROXY_CONFIG_DIR \u003d \"ovn-metadata-proxy\""},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"_HEADER_CONFIG_TEMPLATE \u003d \"\"\""},{"line_number":38,"context_line":"    http-request add-header X-OVN-%(res_type)s-ID %(res_id)s"},{"line_number":39,"context_line":"\"\"\""},{"line_number":40,"context_line":""},{"line_number":41,"context_line":"_UNLIMITED_CONFIG_TEMPLATE \u003d \"\"\""}],"source_content_type":"text/x-python","patch_set":6,"id":"e7e1af2d_944eaf9c","line":38,"updated":"2023-10-25 10:48:19.000000000","message":"I just noticed a difference here while reviewing Brian\u0027s code deduplication patch:\n\nIn the old driver we always delete the header belonging to the other resource. Which may be an anti-spoofing measure, depending on how we process these headers later. Please see here:\n\nhttps://opendev.org/openstack/neutron/src/commit/f185379980d8457d5ea8716eadee3ba56e35cb11/neutron/agent/metadata/driver.py#L52\nhttps://opendev.org/openstack/neutron/src/commit/f185379980d8457d5ea8716eadee3ba56e35cb11/neutron/agent/metadata/driver.py#L129-L130","commit_id":"1bd41edf5b617e61a47984d17feacf755caa7fbf"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"bf934b6d2cc6124f36f50533615e96de9fe4c97b","unresolved":false,"context_lines":[{"line_number":35,"context_line":"PROXY_CONFIG_DIR \u003d \"ovn-metadata-proxy\""},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"_HEADER_CONFIG_TEMPLATE \u003d \"\"\""},{"line_number":38,"context_line":"    http-request add-header X-OVN-%(res_type)s-ID %(res_id)s"},{"line_number":39,"context_line":"\"\"\""},{"line_number":40,"context_line":""},{"line_number":41,"context_line":"_UNLIMITED_CONFIG_TEMPLATE \u003d \"\"\""}],"source_content_type":"text/x-python","patch_set":6,"id":"ebc32607_baa9cdb5","line":38,"in_reply_to":"b04103d3_92c41ff5","updated":"2023-10-30 17:23:07.000000000","message":"Done","commit_id":"1bd41edf5b617e61a47984d17feacf755caa7fbf"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"3e7bb1bcad6bf6dff620a66b35359e8e43bdefb2","unresolved":true,"context_lines":[{"line_number":35,"context_line":"PROXY_CONFIG_DIR \u003d \"ovn-metadata-proxy\""},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"_HEADER_CONFIG_TEMPLATE \u003d \"\"\""},{"line_number":38,"context_line":"    http-request add-header X-OVN-%(res_type)s-ID %(res_id)s"},{"line_number":39,"context_line":"\"\"\""},{"line_number":40,"context_line":""},{"line_number":41,"context_line":"_UNLIMITED_CONFIG_TEMPLATE \u003d \"\"\""}],"source_content_type":"text/x-python","patch_set":6,"id":"b04103d3_92c41ff5","line":38,"in_reply_to":"e7e1af2d_944eaf9c","updated":"2023-10-25 23:26:00.000000000","message":"Hi Bence, thanks for noticing. Luckily I don\u0027t think this is an issue with the OVN code as it shouldn\u0027t every proxy metadata requests from external networks. I have added info to the comment in my follow-on to explain this more.","commit_id":"1bd41edf5b617e61a47984d17feacf755caa7fbf"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"5e7bfd271d8f03f520369de36caa1c5b36d82ddd","unresolved":true,"context_lines":[{"line_number":213,"context_line":"            # state."},{"line_number":214,"context_line":"            ip_lib.IpAddrCommand("},{"line_number":215,"context_line":"                parent\u003dip_lib.IPDevice(name\u003dbind_interface, namespace\u003dns_name)"},{"line_number":216,"context_line":"            ).wait_until_address_ready(address\u003dbind_address_v6)"},{"line_number":217,"context_line":"        pm.enable()"},{"line_number":218,"context_line":"        monitor.register(uuid, METADATA_SERVICE_NAME, pm)"},{"line_number":219,"context_line":"        cls.monitors[router_id] \u003d pm"}],"source_content_type":"text/x-python","patch_set":7,"id":"9d533cb5_0cd3120a","line":216,"updated":"2023-10-30 12:56:51.000000000","message":"Do we want to fail spawning the whole haproxy process if just the IPv6 preparations fail?","commit_id":"bf80fced1b42940e594e9117db9b72bc34271306"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"e6f4d85808e0306dfbdbe00e84d4696ac7082e74","unresolved":false,"context_lines":[{"line_number":213,"context_line":"            # state."},{"line_number":214,"context_line":"            ip_lib.IpAddrCommand("},{"line_number":215,"context_line":"                parent\u003dip_lib.IPDevice(name\u003dbind_interface, namespace\u003dns_name)"},{"line_number":216,"context_line":"            ).wait_until_address_ready(address\u003dbind_address_v6)"},{"line_number":217,"context_line":"        pm.enable()"},{"line_number":218,"context_line":"        monitor.register(uuid, METADATA_SERVICE_NAME, pm)"},{"line_number":219,"context_line":"        cls.monitors[router_id] \u003d pm"}],"source_content_type":"text/x-python","patch_set":7,"id":"15c29162_dfe5265c","line":216,"in_reply_to":"3dab370a_2a9068ec","updated":"2023-11-02 20:56:19.000000000","message":"It\u0027s not just DAD but any type of unexpected failure that leads to the address not being ready, like eg. a bug in the OS. I would suggest to be more defensive here.","commit_id":"bf80fced1b42940e594e9117db9b72bc34271306"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"bf934b6d2cc6124f36f50533615e96de9fe4c97b","unresolved":false,"context_lines":[{"line_number":213,"context_line":"            # state."},{"line_number":214,"context_line":"            ip_lib.IpAddrCommand("},{"line_number":215,"context_line":"                parent\u003dip_lib.IPDevice(name\u003dbind_interface, namespace\u003dns_name)"},{"line_number":216,"context_line":"            ).wait_until_address_ready(address\u003dbind_address_v6)"},{"line_number":217,"context_line":"        pm.enable()"},{"line_number":218,"context_line":"        monitor.register(uuid, METADATA_SERVICE_NAME, pm)"},{"line_number":219,"context_line":"        cls.monitors[router_id] \u003d pm"}],"source_content_type":"text/x-python","patch_set":7,"id":"3dab370a_2a9068ec","line":216,"in_reply_to":"9d533cb5_0cd3120a","updated":"2023-10-30 17:23:07.000000000","message":"I think the answer is No. Rodolfo mentioned this on PS2 and Daniel mentioned DAD failure really can\u0027t happen in this case in OVN.\n\nFWIW, the combined code in my follow-up adds a check and disables IPv6 in the failure case as the non-OVN agent code does that. It basically sets bind_interface and bind_address_v6 to None and continues on...","commit_id":"bf80fced1b42940e594e9117db9b72bc34271306"}],"neutron/agent/ovn/metadata/server.py":[{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"0726e95db276ec2cd1323717e9b1376227003cb8","unresolved":true,"context_lines":[{"line_number":16,"context_line":"import urllib"},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"import netaddr"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"from neutron._i18n import _"},{"line_number":21,"context_line":"from neutron.agent.linux import utils as agent_utils"},{"line_number":22,"context_line":"from neutron.agent.ovn.metadata import ovsdb"},{"line_number":23,"context_line":"from neutron.common import ipv6_utils"},{"line_number":24,"context_line":"from neutron.common.ovn import constants as ovn_const"},{"line_number":25,"context_line":"from neutron.common import utils as common_utils"},{"line_number":26,"context_line":"from neutron.conf.agent.metadata import config"},{"line_number":27,"context_line":"from neutron_lib.callbacks import events"},{"line_number":28,"context_line":"from neutron_lib.callbacks import registry"},{"line_number":29,"context_line":"from neutron_lib.callbacks import resources"},{"line_number":30,"context_line":"from oslo_config import cfg"},{"line_number":31,"context_line":"from oslo_log import log as logging"},{"line_number":32,"context_line":"from oslo_utils import netutils"},{"line_number":33,"context_line":"import requests"},{"line_number":34,"context_line":"import webob"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"LOG \u003d logging.getLogger(__name__)"}],"source_content_type":"text/x-python","patch_set":7,"id":"a075d90c_989fd501","line":34,"range":{"start_line":19,"start_character":0,"end_line":34,"end_character":12},"updated":"2023-10-30 12:49:30.000000000","message":"Maybe we should fix this in our rafactoring patch series 😊","commit_id":"bf80fced1b42940e594e9117db9b72bc34271306"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"bf934b6d2cc6124f36f50533615e96de9fe4c97b","unresolved":true,"context_lines":[{"line_number":16,"context_line":"import urllib"},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"import netaddr"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"from neutron._i18n import _"},{"line_number":21,"context_line":"from neutron.agent.linux import utils as agent_utils"},{"line_number":22,"context_line":"from neutron.agent.ovn.metadata import ovsdb"},{"line_number":23,"context_line":"from neutron.common import ipv6_utils"},{"line_number":24,"context_line":"from neutron.common.ovn import constants as ovn_const"},{"line_number":25,"context_line":"from neutron.common import utils as common_utils"},{"line_number":26,"context_line":"from neutron.conf.agent.metadata import config"},{"line_number":27,"context_line":"from neutron_lib.callbacks import events"},{"line_number":28,"context_line":"from neutron_lib.callbacks import registry"},{"line_number":29,"context_line":"from neutron_lib.callbacks import resources"},{"line_number":30,"context_line":"from oslo_config import cfg"},{"line_number":31,"context_line":"from oslo_log import log as logging"},{"line_number":32,"context_line":"from oslo_utils import netutils"},{"line_number":33,"context_line":"import requests"},{"line_number":34,"context_line":"import webob"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"LOG \u003d logging.getLogger(__name__)"}],"source_content_type":"text/x-python","patch_set":7,"id":"ee7a11bb_c0cc019b","line":34,"range":{"start_line":19,"start_character":0,"end_line":34,"end_character":12},"in_reply_to":"a075d90c_989fd501","updated":"2023-10-30 17:23:07.000000000","message":"Sure, just tell me what we need to fix as I can\u0027t tell what is wrong.","commit_id":"bf80fced1b42940e594e9117db9b72bc34271306"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"e3d83df65599284ae79bb41190872e2f6c139f0c","unresolved":true,"context_lines":[{"line_number":102,"context_line":"        network_id \u003d req.headers.get(\u0027X-OVN-Network-ID\u0027)"},{"line_number":103,"context_line":""},{"line_number":104,"context_line":"        remote_ip \u003d netaddr.IPAddress(remote_address)"},{"line_number":105,"context_line":"        if remote_ip.is_link_local():"},{"line_number":106,"context_line":"            # When haproxy sees an ipv6 link-local client address"},{"line_number":107,"context_line":"            # (and sends that to us in X-Forwarded-For) we must rely"},{"line_number":108,"context_line":"            # on the EUI encoded in it, because that\u0027s all we can"}],"source_content_type":"text/x-python","patch_set":7,"id":"9ac86417_f8f36e97","line":105,"updated":"2023-10-31 21:56:12.000000000","message":"(No action required) Looking at the fork code for \"non-OVN\" agent, is this part of the code relevant here too, considering that the patch apparently makes haproxy bind for both protocols?\n\n```\n            if remote_ip.is_ipv4_mapped():\n                # When haproxy listens on v4 AND v6 then it inserts ipv4\n                # addresses as ipv4-mapped v6 addresses into X-Forwarded-For.\n                forwarded_for \u003d str(remote_ip.ipv4())\n```","commit_id":"bf80fced1b42940e594e9117db9b72bc34271306"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"fdbff5f4f30d825f6f0b1584a255197089fc8f9f","unresolved":true,"context_lines":[{"line_number":102,"context_line":"        network_id \u003d req.headers.get(\u0027X-OVN-Network-ID\u0027)"},{"line_number":103,"context_line":""},{"line_number":104,"context_line":"        remote_ip \u003d netaddr.IPAddress(remote_address)"},{"line_number":105,"context_line":"        if remote_ip.is_link_local():"},{"line_number":106,"context_line":"            # When haproxy sees an ipv6 link-local client address"},{"line_number":107,"context_line":"            # (and sends that to us in X-Forwarded-For) we must rely"},{"line_number":108,"context_line":"            # on the EUI encoded in it, because that\u0027s all we can"}],"source_content_type":"text/x-python","patch_set":7,"id":"736b29dc_37a5f4b9","line":105,"in_reply_to":"4422d70d_b9ed4d48","updated":"2023-11-05 23:46:41.000000000","message":"So I started updating this and it looks like from reading the ovsdb code functional tests, the ports found via get_network_port_bindings_by_ip() from ovsdb will have port.mac \u003d\u003d [MAC IP] ? That\u0027s from _create_bound_port_with_ip()\n\nHave to verify but not what I expected.","commit_id":"bf80fced1b42940e594e9117db9b72bc34271306"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"074b405b00133c3244eec74196af182291a95a0d","unresolved":true,"context_lines":[{"line_number":102,"context_line":"        network_id \u003d req.headers.get(\u0027X-OVN-Network-ID\u0027)"},{"line_number":103,"context_line":""},{"line_number":104,"context_line":"        remote_ip \u003d netaddr.IPAddress(remote_address)"},{"line_number":105,"context_line":"        if remote_ip.is_link_local():"},{"line_number":106,"context_line":"            # When haproxy sees an ipv6 link-local client address"},{"line_number":107,"context_line":"            # (and sends that to us in X-Forwarded-For) we must rely"},{"line_number":108,"context_line":"            # on the EUI encoded in it, because that\u0027s all we can"}],"source_content_type":"text/x-python","patch_set":7,"id":"e603aad7_b3ec3285","line":105,"in_reply_to":"736b29dc_37a5f4b9","updated":"2023-11-06 21:49:00.000000000","message":"So to answer my own question, this is the mac address field of a Port_Binding entry:\n\nmac: [\"MAC IP {IP2..IPN}\"]\n\nFor example:\n\nmac: [\"fa:16:3e:3c:85:ec 10.0.0.2 fd19:b913:a708:0:f816:3eff:fe3c:85ec\"]\n\nBut at least on my system I don\u0027t see one with an IPv6 link-local address :-/","commit_id":"bf80fced1b42940e594e9117db9b72bc34271306"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"c2ba8d0c452193134c5df5aada89740f726ab9bd","unresolved":true,"context_lines":[{"line_number":102,"context_line":"        network_id \u003d req.headers.get(\u0027X-OVN-Network-ID\u0027)"},{"line_number":103,"context_line":""},{"line_number":104,"context_line":"        remote_ip \u003d netaddr.IPAddress(remote_address)"},{"line_number":105,"context_line":"        if remote_ip.is_link_local():"},{"line_number":106,"context_line":"            # When haproxy sees an ipv6 link-local client address"},{"line_number":107,"context_line":"            # (and sends that to us in X-Forwarded-For) we must rely"},{"line_number":108,"context_line":"            # on the EUI encoded in it, because that\u0027s all we can"}],"source_content_type":"text/x-python","patch_set":7,"id":"4422d70d_b9ed4d48","line":105,"in_reply_to":"9ac86417_f8f36e97","updated":"2023-11-01 01:47:21.000000000","message":"It is a good question and I don\u0027t know the answer. Seems it should behave similarly - using the MAC as an optional parameter to match, and all this should be under an \"this is IPv6\" check.\n\nAnd I\u0027ll admit having different names here versus the other code only make this more confusing.\n\nI\u0027ll take a look at updating this tomorrow.","commit_id":"bf80fced1b42940e594e9117db9b72bc34271306"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"82bc326b4a6783f52770db20695629cddd98eee1","unresolved":false,"context_lines":[{"line_number":102,"context_line":"        network_id \u003d req.headers.get(\u0027X-OVN-Network-ID\u0027)"},{"line_number":103,"context_line":""},{"line_number":104,"context_line":"        remote_ip \u003d netaddr.IPAddress(remote_address)"},{"line_number":105,"context_line":"        if remote_ip.is_link_local():"},{"line_number":106,"context_line":"            # When haproxy sees an ipv6 link-local client address"},{"line_number":107,"context_line":"            # (and sends that to us in X-Forwarded-For) we must rely"},{"line_number":108,"context_line":"            # on the EUI encoded in it, because that\u0027s all we can"}],"source_content_type":"text/x-python","patch_set":7,"id":"1d2272a4_6616bfd2","line":105,"in_reply_to":"e603aad7_b3ec3285","updated":"2023-11-06 21:54:09.000000000","message":"Done","commit_id":"bf80fced1b42940e594e9117db9b72bc34271306"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"e3d83df65599284ae79bb41190872e2f6c139f0c","unresolved":true,"context_lines":[{"line_number":109,"context_line":"            # recognize."},{"line_number":110,"context_line":"            remote_address \u003d str(netutils.get_mac_addr_by_ipv6(remote_ip))"},{"line_number":111,"context_line":""},{"line_number":112,"context_line":"        ports \u003d self.sb_idl.get_network_port_bindings_by_ip(network_id,"},{"line_number":113,"context_line":"                                                            remote_address)"},{"line_number":114,"context_line":"        num_ports \u003d len(ports)"},{"line_number":115,"context_line":"        if num_ports \u003d\u003d 1:"}],"source_content_type":"text/x-python","patch_set":7,"id":"a4ce87fc_028236b4","line":112,"updated":"2023-10-31 21:56:12.000000000","message":"(No action required) this endpoint name is unfortunate since it supports any type of addresses, incl. MAC addresses.","commit_id":"bf80fced1b42940e594e9117db9b72bc34271306"}],"neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/impl_idl_ovn.py":[{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"b5f76eb5290ce92b3bbd634051612b3df17705e4","unresolved":true,"context_lines":[{"line_number":963,"context_line":"            # [\"MAC IP {IP2...IPN}\"]"},{"line_number":964,"context_line":"            mac_ip \u003d port.mac[0].split(\u0027 \u0027)"},{"line_number":965,"context_line":"            if mac and mac not in mac_ip:"},{"line_number":966,"context_line":"                return False"},{"line_number":967,"context_line":""},{"line_number":968,"context_line":"            is_in_network \u003d utils.get_network_name_from_datapath("},{"line_number":969,"context_line":"                port.datapath) \u003d\u003d network"}],"source_content_type":"text/x-python","patch_set":10,"id":"88af24bc_d3899c87","line":966,"updated":"2023-11-30 19:20:26.000000000","message":"So I found the problem Bence :)  Daniel\u0027s original change passed either an IP address or a MAC in the ip_address argument, and we only care if one is present, not both. When I changed this slightly in my test env it worked. I\u0027ll push an update.","commit_id":"aee2930583790ae87636230d81022d8ee626a1c9"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"a39d63355d50052beec2183a6f7dbeb1d4a71f04","unresolved":false,"context_lines":[{"line_number":963,"context_line":"            # [\"MAC IP {IP2...IPN}\"]"},{"line_number":964,"context_line":"            mac_ip \u003d port.mac[0].split(\u0027 \u0027)"},{"line_number":965,"context_line":"            if mac and mac not in mac_ip:"},{"line_number":966,"context_line":"                return False"},{"line_number":967,"context_line":""},{"line_number":968,"context_line":"            is_in_network \u003d utils.get_network_name_from_datapath("},{"line_number":969,"context_line":"                port.datapath) \u003d\u003d network"}],"source_content_type":"text/x-python","patch_set":10,"id":"8eae71d3_07935334","line":966,"in_reply_to":"88af24bc_d3899c87","updated":"2023-12-08 22:19:40.000000000","message":"Done","commit_id":"aee2930583790ae87636230d81022d8ee626a1c9"},{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"working for Ericsson, UTC+1 (+DST)"},"change_message_id":"0e445995748301802539d5bbcffa2b9da590b935","unresolved":false,"context_lines":[{"line_number":963,"context_line":"            # [\"MAC IP {IP2...IPN}\"]"},{"line_number":964,"context_line":"            mac_ip \u003d port.mac[0].split(\u0027 \u0027)"},{"line_number":965,"context_line":"            if mac and mac not in mac_ip:"},{"line_number":966,"context_line":"                return False"},{"line_number":967,"context_line":""},{"line_number":968,"context_line":"            is_in_network \u003d utils.get_network_name_from_datapath("},{"line_number":969,"context_line":"                port.datapath) \u003d\u003d network"}],"source_content_type":"text/x-python","patch_set":10,"id":"b1fab4f4_1ecffd86","line":966,"in_reply_to":"88af24bc_d3899c87","updated":"2023-12-04 15:06:13.000000000","message":"Thanks! ps11 is working in my environment too. I guess from the followup patch that turns on a tempest test for ipv6 metadata on ovn, we\u0027ll have a test that would have caught this.","commit_id":"aee2930583790ae87636230d81022d8ee626a1c9"}],"neutron/tests/unit/agent/ovn/metadata/test_agent.py":[{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"ede7a2a3e055c3c0830311e3cf94ebe5c2084cb4","unresolved":true,"context_lines":[{"line_number":33,"context_line":"from neutron.conf.plugins.ml2.drivers.ovn import ovn_conf"},{"line_number":34,"context_line":"from neutron.tests import base"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"METADATA_V6_CIDR \u003d n_const.METADATA_V6_IP + \u0027/64\u0027"},{"line_number":37,"context_line":"OvnPortInfo \u003d collections.namedtuple("},{"line_number":38,"context_line":"    \u0027OvnPortInfo\u0027, [\u0027datapath\u0027, \u0027type\u0027, \u0027mac\u0027, \u0027external_ids\u0027, \u0027logical_port\u0027])"},{"line_number":39,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"0281c186_b1953616","line":36,"updated":"2023-09-07 14:11:41.000000000","message":"Same commend about using neutron-lib constant.","commit_id":"fb154c6663b6b60fd417511c0100847d0202d44b"},{"author":{"_account_id":23804,"name":"Daniel Alvarez","email":"dalvarez@redhat.com","username":"dalvarez"},"change_message_id":"5fdb29b7db2f8187960459e2a2ad62db60729c4d","unresolved":true,"context_lines":[{"line_number":33,"context_line":"from neutron.conf.plugins.ml2.drivers.ovn import ovn_conf"},{"line_number":34,"context_line":"from neutron.tests import base"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"METADATA_V6_CIDR \u003d n_const.METADATA_V6_IP + \u0027/64\u0027"},{"line_number":37,"context_line":"OvnPortInfo \u003d collections.namedtuple("},{"line_number":38,"context_line":"    \u0027OvnPortInfo\u0027, [\u0027datapath\u0027, \u0027type\u0027, \u0027mac\u0027, \u0027external_ids\u0027, \u0027logical_port\u0027])"},{"line_number":39,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"b19b00cc_54a6f18d","line":36,"in_reply_to":"0281c186_b1953616","updated":"2023-09-07 15:26:50.000000000","message":"please see above","commit_id":"fb154c6663b6b60fd417511c0100847d0202d44b"},{"author":{"_account_id":23804,"name":"Daniel Alvarez","email":"dalvarez@redhat.com","username":"dalvarez"},"change_message_id":"8d125e7d83598660bcac8938800d7849f7ab2ba2","unresolved":false,"context_lines":[{"line_number":33,"context_line":"from neutron.conf.plugins.ml2.drivers.ovn import ovn_conf"},{"line_number":34,"context_line":"from neutron.tests import base"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"METADATA_V6_CIDR \u003d n_const.METADATA_V6_IP + \u0027/64\u0027"},{"line_number":37,"context_line":"OvnPortInfo \u003d collections.namedtuple("},{"line_number":38,"context_line":"    \u0027OvnPortInfo\u0027, [\u0027datapath\u0027, \u0027type\u0027, \u0027mac\u0027, \u0027external_ids\u0027, \u0027logical_port\u0027])"},{"line_number":39,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"aa989886_58b83982","line":36,"in_reply_to":"b19b00cc_54a6f18d","updated":"2023-09-13 15:59:46.000000000","message":"Done","commit_id":"fb154c6663b6b60fd417511c0100847d0202d44b"}]}