)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"219123512d2c47dd8f9317819cc7e0537f5af92b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"49836764_febdb2a2","updated":"2025-12-11 17:45:19.000000000","message":"This patch makes me happy.😊","commit_id":"5ffa8572e592a731c452550e7eba179601938785"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"3af23ce221f1c23449e25fe0a0fb25eb4180bae1","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"9b8df997_83230ace","in_reply_to":"49836764_febdb2a2","updated":"2025-12-11 20:01:13.000000000","message":"A Christmas gift from me to you. 🎁","commit_id":"5ffa8572e592a731c452550e7eba179601938785"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"834244c5e3666207e60e38bbb67da3818bd12de1","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"ead5647d_7d649ec4","updated":"2025-12-11 22:27:28.000000000","message":"Overall I like this approach, and it also simplifies the code.","commit_id":"014f4e592bec02f2def5af2dfc14c4f9fb186c7e"}],"neutron/services/bgp/commands.py":[{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"219123512d2c47dd8f9317819cc7e0537f5af92b","unresolved":true,"context_lines":[{"line_number":227,"context_line":""},{"line_number":228,"context_line":"    @property"},{"line_number":229,"context_line":"    def all_chassis(self):"},{"line_number":230,"context_line":"        return [rowview.RowView(r)"},{"line_number":231,"context_line":"                for r in self.sb_api.tables[\u0027Chassis_Private\u0027].rows.values()]"},{"line_number":232,"context_line":""},{"line_number":233,"context_line":"    def reconcile_all_chassis(self, txn):"}],"source_content_type":"text/x-python","patch_set":1,"id":"17bd75e5_04ff7559","line":230,"updated":"2025-12-11 17:45:19.000000000","message":"nit: If all_chassis is only used w/in this command (or other commands), we probably don\u0027t need to iterate over all of them to wrap them w/ a RowView since it mostly exists to keep non-Command code from modifying columns.","commit_id":"5ffa8572e592a731c452550e7eba179601938785"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"3af23ce221f1c23449e25fe0a0fb25eb4180bae1","unresolved":false,"context_lines":[{"line_number":227,"context_line":""},{"line_number":228,"context_line":"    @property"},{"line_number":229,"context_line":"    def all_chassis(self):"},{"line_number":230,"context_line":"        return [rowview.RowView(r)"},{"line_number":231,"context_line":"                for r in self.sb_api.tables[\u0027Chassis_Private\u0027].rows.values()]"},{"line_number":232,"context_line":""},{"line_number":233,"context_line":"    def reconcile_all_chassis(self, txn):"}],"source_content_type":"text/x-python","patch_set":1,"id":"8b2206df_da3f7eba","line":230,"in_reply_to":"17bd75e5_04ff7559","updated":"2025-12-11 20:01:13.000000000","message":"Done","commit_id":"5ffa8572e592a731c452550e7eba179601938785"}],"neutron/services/bgp/helpers.py":[{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"e387e9f2c5b45f82560697a5437a7a24afbf3763","unresolved":true,"context_lines":[{"line_number":43,"context_line":"    # 4. Format into standard MAC string (XX:XX:XX:XX:XX:XX)"},{"line_number":44,"context_line":"    mac_address \u003d \u0027:\u0027.join(f\u0027{b:02x}\u0027 for b in mac_bytes)"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"    return mac_address"},{"line_number":47,"context_line":""},{"line_number":48,"context_line":""},{"line_number":49,"context_line":"def get_all_chassis(sb_ovn):"}],"source_content_type":"text/x-python","patch_set":1,"id":"d5a9635c_4aeb6d6e","line":46,"updated":"2025-12-11 08:15:50.000000000","message":"I have one question/doubt about this change. IIUC those MAC addresses are used for the connection between ToR switches (this /30 networks which you were showing us, right?). With previous approach we had constant prefix of the MAC which could maybe help with debugging connectivity issues in the real world as you could potentially quickly identify that some packets are comming from one of the ovn chassis bgp routers by just looking at the begin of the MAC address.\nNow, with this new approach, the whole MAC is random if I\u0027m not mistaken, so there is no way to check it that way. In theory we could even set it so that we would \"fake\" some other vendors MAC address, correct?","commit_id":"5ffa8572e592a731c452550e7eba179601938785"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"697dd405bb30a5bad1ebeabc00c8d2d964b90018","unresolved":true,"context_lines":[{"line_number":43,"context_line":"    # 4. Format into standard MAC string (XX:XX:XX:XX:XX:XX)"},{"line_number":44,"context_line":"    mac_address \u003d \u0027:\u0027.join(f\u0027{b:02x}\u0027 for b in mac_bytes)"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"    return mac_address"},{"line_number":47,"context_line":""},{"line_number":48,"context_line":""},{"line_number":49,"context_line":"def get_all_chassis(sb_ovn):"}],"source_content_type":"text/x-python","patch_set":1,"id":"cb7d7f47_e774b1bb","line":46,"in_reply_to":"a42afd56_6969869d","updated":"2025-12-12 15:45:41.000000000","message":"We can, however what piece of information would that give you? You\u0027ll see what was generated in the OVN DB LRP row, unless you want to troubleshoot that someone has not changed the MAC between the generation code and the commit.","commit_id":"5ffa8572e592a731c452550e7eba179601938785"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"78f171fd3a0b28534437b9758c5b9e26ae719e93","unresolved":false,"context_lines":[{"line_number":43,"context_line":"    # 4. Format into standard MAC string (XX:XX:XX:XX:XX:XX)"},{"line_number":44,"context_line":"    mac_address \u003d \u0027:\u0027.join(f\u0027{b:02x}\u0027 for b in mac_bytes)"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"    return mac_address"},{"line_number":47,"context_line":""},{"line_number":48,"context_line":""},{"line_number":49,"context_line":"def get_all_chassis(sb_ovn):"}],"source_content_type":"text/x-python","patch_set":1,"id":"65552869_ea63a5c5","line":46,"in_reply_to":"cb7d7f47_e774b1bb","updated":"2025-12-15 14:28:03.000000000","message":"@Kuba: thx for explanation. I\u0027m ok with it. Just wanted to make sure that this was considered and it will be fine like that (and that I understand it correctly 😊)\n@Miro: such log message will most likely be already gone when it would be really needed 😊","commit_id":"5ffa8572e592a731c452550e7eba179601938785"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"7129a3e6ad132805d1e2d2cdbbf32203128886d4","unresolved":true,"context_lines":[{"line_number":43,"context_line":"    # 4. Format into standard MAC string (XX:XX:XX:XX:XX:XX)"},{"line_number":44,"context_line":"    mac_address \u003d \u0027:\u0027.join(f\u0027{b:02x}\u0027 for b in mac_bytes)"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"    return mac_address"},{"line_number":47,"context_line":""},{"line_number":48,"context_line":""},{"line_number":49,"context_line":"def get_all_chassis(sb_ovn):"}],"source_content_type":"text/x-python","patch_set":1,"id":"f237b9dd_629d11c6","line":46,"in_reply_to":"d5a9635c_4aeb6d6e","updated":"2025-12-11 13:03:28.000000000","message":"Thanks for looking at it and being concerned!\n\nI may need to explain this better in the commit message. The MAC is for every LRP in the \"BGP world\" - so all the way from the LRPs that are connected to the leaf switches, LRPs that connect the per-chassis routers with the BGP distributed router and LRPs that connect to the \"Neutron world\".\n\nIt is true it may be easier for troubleshooting - now the MAC is based on the system-id of a chassis - or the ID of a Neutron resource - but you can\u0027t read it easily from the MAC itself.\n\nAs for the vendor MAC - it is true it may happen, however the L2 networks we use between the routers are very small so this MAC never gets out - except the LRP that connects to a chassis. Maybe we could rewrite the source MAC by the openflow later for egress traffic to be on a safe side.\n\nAs Terry described in the previous PS - there is also a very small chance we may generate the same MAC as we\u0027re always using 48 bit hash. It also should be fine to have two same MACs as long as they are not on the same L2 domain as we use routing everywhere.\n\nThere are some tradeoffs, that\u0027s why I sent it as a separate patch so we have better insight in what changes in the code and we can discuss in detail.","commit_id":"5ffa8572e592a731c452550e7eba179601938785"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"52334961d65465f0508a85dd0c218b90f7eee792","unresolved":true,"context_lines":[{"line_number":43,"context_line":"    # 4. Format into standard MAC string (XX:XX:XX:XX:XX:XX)"},{"line_number":44,"context_line":"    mac_address \u003d \u0027:\u0027.join(f\u0027{b:02x}\u0027 for b in mac_bytes)"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"    return mac_address"},{"line_number":47,"context_line":""},{"line_number":48,"context_line":""},{"line_number":49,"context_line":"def get_all_chassis(sb_ovn):"}],"source_content_type":"text/x-python","patch_set":1,"id":"a42afd56_6969869d","line":46,"in_reply_to":"f237b9dd_629d11c6","updated":"2025-12-12 15:20:21.000000000","message":"I know this is not much. But how about logging it for debugging purposes?\n```\nLOG.debug(f\u0027Generated {mac_address} from {lrp_name}\u0027)\n```","commit_id":"5ffa8572e592a731c452550e7eba179601938785"}],"neutron/tests/unit/services/bgp/test_helpers.py":[{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"834244c5e3666207e60e38bbb67da3818bd12de1","unresolved":true,"context_lines":[{"line_number":26,"context_line":"        for i in range(num_macs):"},{"line_number":27,"context_line":"            lrp_name \u003d f\u0027test-lrp-{i}\u0027"},{"line_number":28,"context_line":"            mac \u003d helpers.get_mac_address_from_lrp_name(lrp_name)"},{"line_number":29,"context_line":"            # make sure mac is valid"},{"line_number":30,"context_line":"            netaddr.EUI(mac, version\u003d48)"},{"line_number":31,"context_line":"            macs.add(mac)"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"        self.assertEqual(num_macs, len(macs))"}],"source_content_type":"text/x-python","patch_set":2,"id":"4517a7f0_30e0d73b","line":30,"range":{"start_line":29,"start_character":12,"end_line":30,"end_character":40},"updated":"2025-12-11 22:27:28.000000000","message":"Nit, instead of a comment, use test approach and `assert` on things that should happen.\nConsider:\n```\n            self.assertIsInstance(\n                netaddr.EUI(mac, version\u003d48),\n                netaddr.EUI,\n                f\"{mac} is not a valid MAC address\"\n            )\n```","commit_id":"014f4e592bec02f2def5af2dfc14c4f9fb186c7e"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"697dd405bb30a5bad1ebeabc00c8d2d964b90018","unresolved":false,"context_lines":[{"line_number":26,"context_line":"        for i in range(num_macs):"},{"line_number":27,"context_line":"            lrp_name \u003d f\u0027test-lrp-{i}\u0027"},{"line_number":28,"context_line":"            mac \u003d helpers.get_mac_address_from_lrp_name(lrp_name)"},{"line_number":29,"context_line":"            # make sure mac is valid"},{"line_number":30,"context_line":"            netaddr.EUI(mac, version\u003d48)"},{"line_number":31,"context_line":"            macs.add(mac)"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"        self.assertEqual(num_macs, len(macs))"}],"source_content_type":"text/x-python","patch_set":2,"id":"84e5195e_ac66bb81","line":30,"range":{"start_line":29,"start_character":12,"end_line":30,"end_character":40},"in_reply_to":"018a5966_480c3b92","updated":"2025-12-12 15:45:41.000000000","message":"Done","commit_id":"014f4e592bec02f2def5af2dfc14c4f9fb186c7e"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"7badd34e1ab373225f1878caef943e4816f31431","unresolved":false,"context_lines":[{"line_number":26,"context_line":"        for i in range(num_macs):"},{"line_number":27,"context_line":"            lrp_name \u003d f\u0027test-lrp-{i}\u0027"},{"line_number":28,"context_line":"            mac \u003d helpers.get_mac_address_from_lrp_name(lrp_name)"},{"line_number":29,"context_line":"            # make sure mac is valid"},{"line_number":30,"context_line":"            netaddr.EUI(mac, version\u003d48)"},{"line_number":31,"context_line":"            macs.add(mac)"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"        self.assertEqual(num_macs, len(macs))"}],"source_content_type":"text/x-python","patch_set":2,"id":"c60e5792_8fc35b61","line":30,"range":{"start_line":29,"start_character":12,"end_line":30,"end_character":40},"in_reply_to":"4517a7f0_30e0d73b","updated":"2025-12-11 22:27:41.000000000","message":"Done","commit_id":"014f4e592bec02f2def5af2dfc14c4f9fb186c7e"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"52334961d65465f0508a85dd0c218b90f7eee792","unresolved":true,"context_lines":[{"line_number":26,"context_line":"        for i in range(num_macs):"},{"line_number":27,"context_line":"            lrp_name \u003d f\u0027test-lrp-{i}\u0027"},{"line_number":28,"context_line":"            mac \u003d helpers.get_mac_address_from_lrp_name(lrp_name)"},{"line_number":29,"context_line":"            # make sure mac is valid"},{"line_number":30,"context_line":"            netaddr.EUI(mac, version\u003d48)"},{"line_number":31,"context_line":"            macs.add(mac)"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"        self.assertEqual(num_macs, len(macs))"}],"source_content_type":"text/x-python","patch_set":2,"id":"018a5966_480c3b92","line":30,"range":{"start_line":29,"start_character":12,"end_line":30,"end_character":40},"in_reply_to":"85d4a8e6_2984949b","updated":"2025-12-12 15:20:21.000000000","message":"Hmmm I see, I thought that would work as it would raise and not create an instance.\nMaybe something like this?:\n```\ntry:\n    netaddr.EUI(mac, version\u003d48)\nexcept netaddr.AddrFormatError as e:\n    self.fail(f\"Invalid MAC address generated: {mac}, error: {e}\")\n```\nMy whole point here is to simply express what your intention is with code instead of a comment. But if it gets too messy, feel free to drop it. it was just nit","commit_id":"014f4e592bec02f2def5af2dfc14c4f9fb186c7e"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"c302694b907987ed67c6df92963259c8266fe9d7","unresolved":true,"context_lines":[{"line_number":26,"context_line":"        for i in range(num_macs):"},{"line_number":27,"context_line":"            lrp_name \u003d f\u0027test-lrp-{i}\u0027"},{"line_number":28,"context_line":"            mac \u003d helpers.get_mac_address_from_lrp_name(lrp_name)"},{"line_number":29,"context_line":"            # make sure mac is valid"},{"line_number":30,"context_line":"            netaddr.EUI(mac, version\u003d48)"},{"line_number":31,"context_line":"            macs.add(mac)"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"        self.assertEqual(num_macs, len(macs))"}],"source_content_type":"text/x-python","patch_set":2,"id":"85d4a8e6_2984949b","line":30,"range":{"start_line":29,"start_character":12,"end_line":30,"end_character":40},"in_reply_to":"acf37b28_26829591","updated":"2025-12-12 13:16:46.000000000","message":"This doesn\u0027t work, does it? If I pass an invalid MAC to EUI() then it raises an exception right away and we never get to call the assert anyways.","commit_id":"014f4e592bec02f2def5af2dfc14c4f9fb186c7e"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"1542b5d6a960fd4ca5f36c619355f0ea5462bff0","unresolved":true,"context_lines":[{"line_number":26,"context_line":"        for i in range(num_macs):"},{"line_number":27,"context_line":"            lrp_name \u003d f\u0027test-lrp-{i}\u0027"},{"line_number":28,"context_line":"            mac \u003d helpers.get_mac_address_from_lrp_name(lrp_name)"},{"line_number":29,"context_line":"            # make sure mac is valid"},{"line_number":30,"context_line":"            netaddr.EUI(mac, version\u003d48)"},{"line_number":31,"context_line":"            macs.add(mac)"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"        self.assertEqual(num_macs, len(macs))"}],"source_content_type":"text/x-python","patch_set":2,"id":"acf37b28_26829591","line":30,"range":{"start_line":29,"start_character":12,"end_line":30,"end_character":40},"in_reply_to":"c60e5792_8fc35b61","updated":"2025-12-11 22:28:58.000000000","message":"premature \"done\" 😊, sorry","commit_id":"014f4e592bec02f2def5af2dfc14c4f9fb186c7e"}]}