)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"703b4f4ef905b0ca15a77aa62c3026a506309dcd","unresolved":true,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"[DVR] Set arp entries only for single IPs given as allowed addr pair"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"In allowed address pairs of the port there can be given not single IP"},{"line_number":10,"context_line":"address but whole CIDR. In such case ARP entries for IPs from such"},{"line_number":11,"context_line":"cidr will not be added in the DVR router namespace."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Closes-Bug: #1934912"},{"line_number":14,"context_line":"Change-Id: I7bdefea943379125f93b116bb899446b874d9505"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"f384e87c_3387066b","line":12,"range":{"start_line":9,"start_character":0,"end_line":12,"end_character":0},"updated":"2021-08-04 14:51:30.000000000","message":"Then what happens? Let the kernel/namespace/client learn the MAC address of each one IP in the CIDR?","commit_id":"a5e1e6155378910f47c8f2f60c9d950ed29db863"},{"author":{"_account_id":8313,"name":"Lajos Katona","display_name":"lajoskatona","email":"katonalala@gmail.com","username":"elajkat","status":"Ericsson Software Technology"},"change_message_id":"59355dea0ffe5d1db8916e9495136461aa91666b","unresolved":true,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"[DVR] Set arp entries only for single IPs given as allowed addr pair"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"In allowed address pairs of the port there can be given not single IP"},{"line_number":10,"context_line":"address but whole CIDR. In such case ARP entries for IPs from such"},{"line_number":11,"context_line":"cidr will not be added in the DVR router namespace."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Closes-Bug: #1934912"},{"line_number":14,"context_line":"Change-Id: I7bdefea943379125f93b116bb899446b874d9505"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"07ec2d1c_b59cf81e","line":12,"range":{"start_line":9,"start_character":0,"end_line":12,"end_character":0},"in_reply_to":"f384e87c_3387066b","updated":"2021-08-09 13:26:27.000000000","message":"The api-ref is not helping much:\nhttps://opendev.org/openstack/neutron-lib/src/branch/master/api-ref/source/v2/parameters.yaml#L1650-L1658\nbut with this /32 will be allowed for DVR.\nPerhaps a release-notes for it to let users know that from now /32 is allowd as ip of allowed-address-pair for DVR","commit_id":"a5e1e6155378910f47c8f2f60c9d950ed29db863"}],"neutron/agent/l3/dvr_local_router.py":[{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"37a8bf9883d835c95282768e455462bd1f002bd5","unresolved":true,"context_lines":[{"line_number":330,"context_line":"                                           device\u003ddevice,"},{"line_number":331,"context_line":"                                           device_exists\u003ddevice_exists)"},{"line_number":332,"context_line":"                for allowed_address_pair in p.get(\u0027allowed_address_pairs\u0027, []):"},{"line_number":333,"context_line":"                    allowed_address_pair_net \u003d netaddr.IPNetwork("},{"line_number":334,"context_line":"                        allowed_address_pair[\u0027ip_address\u0027])"},{"line_number":335,"context_line":"                    for ip_address in allowed_address_pair_net:"},{"line_number":336,"context_line":"                        self._update_arp_entry("},{"line_number":337,"context_line":"                            str(ip_address),"}],"source_content_type":"text/x-python","patch_set":1,"id":"f5cdf499_42491e16","line":334,"range":{"start_line":333,"start_character":47,"end_line":334,"end_character":59},"updated":"2021-07-08 15:58:20.000000000","message":"What about 0.0.0.0/0 to allowed address pair? Mission impossible...","commit_id":"73843e506cf433dd487f56bb50ed35eeeecc5003"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"26b8ca7e9b2b68f789b02e4f41afe08037a37600","unresolved":true,"context_lines":[{"line_number":330,"context_line":"                                           device\u003ddevice,"},{"line_number":331,"context_line":"                                           device_exists\u003ddevice_exists)"},{"line_number":332,"context_line":"                for allowed_address_pair in p.get(\u0027allowed_address_pairs\u0027, []):"},{"line_number":333,"context_line":"                    allowed_address_pair_net \u003d netaddr.IPNetwork("},{"line_number":334,"context_line":"                        allowed_address_pair[\u0027ip_address\u0027])"},{"line_number":335,"context_line":"                    for ip_address in allowed_address_pair_net:"},{"line_number":336,"context_line":"                        self._update_arp_entry("},{"line_number":337,"context_line":"                            str(ip_address),"}],"source_content_type":"text/x-python","patch_set":1,"id":"8c29bb28_db8516e5","line":334,"range":{"start_line":333,"start_character":47,"end_line":334,"end_character":59},"in_reply_to":"e89f7dcf_58839c0e","updated":"2021-08-03 13:35:06.000000000","message":"You are both right. Given that and also the fact that in ML2/OVN behaviour is similar (VIPs given as whole CIDR don\u0027t works properly and single IP addresses has to be configured to make it working) I changed that patch.\nIn fact the only improvement done here is that now it will not put ugly stacktrace in L3 agent\u0027s logs when CIDR is given as allowed_address_pair.","commit_id":"73843e506cf433dd487f56bb50ed35eeeecc5003"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"d1764f0c04d7f166d7aafd633f65765b6e39bdf3","unresolved":true,"context_lines":[{"line_number":330,"context_line":"                                           device\u003ddevice,"},{"line_number":331,"context_line":"                                           device_exists\u003ddevice_exists)"},{"line_number":332,"context_line":"                for allowed_address_pair in p.get(\u0027allowed_address_pairs\u0027, []):"},{"line_number":333,"context_line":"                    allowed_address_pair_net \u003d netaddr.IPNetwork("},{"line_number":334,"context_line":"                        allowed_address_pair[\u0027ip_address\u0027])"},{"line_number":335,"context_line":"                    for ip_address in allowed_address_pair_net:"},{"line_number":336,"context_line":"                        self._update_arp_entry("},{"line_number":337,"context_line":"                            str(ip_address),"}],"source_content_type":"text/x-python","patch_set":1,"id":"e89f7dcf_58839c0e","line":334,"range":{"start_line":333,"start_character":47,"end_line":334,"end_character":59},"in_reply_to":"f5cdf499_42491e16","updated":"2021-07-09 15:32:47.000000000","message":"Although this is a valid entry (same as 1.0.0.0/8 for example), any allowed address pair with a mask smaller to 16 (that implies 64K addresses), should be limited. Of course, this limit should be discussed. But the point is that we can\u0027t provide this functionality, with the current architecture, for such big CIDRs and we need to raise a warning for this.","commit_id":"73843e506cf433dd487f56bb50ed35eeeecc5003"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"abfcc8cb8e75238ac51f0299e0ee5a1db5d7ff8a","unresolved":true,"context_lines":[{"line_number":332,"context_line":"                for allowed_address_pair in p.get(\u0027allowed_address_pairs\u0027, []):"},{"line_number":333,"context_line":"                    allowed_address_pair_net \u003d netaddr.IPNetwork("},{"line_number":334,"context_line":"                        allowed_address_pair[\u0027ip_address\u0027])"},{"line_number":335,"context_line":"                    for ip_address in allowed_address_pair_net:"},{"line_number":336,"context_line":"                        self._update_arp_entry("},{"line_number":337,"context_line":"                            str(ip_address),"},{"line_number":338,"context_line":"                            allowed_address_pair[\u0027mac_address\u0027],"}],"source_content_type":"text/x-python","patch_set":1,"id":"d55db39d_a8bd9fa9","line":335,"range":{"start_line":335,"start_character":20,"end_line":335,"end_character":62},"updated":"2021-07-08 15:15:09.000000000","message":"I see the problem here and I agree that this could be the solution.\n\nMy concern here: what if allowed_address_pair[\u0027ip_address\u0027] is a /16 CIDR? That means 64K addresses. That will (1) take a lot of time and (2) will overpopulate the ARP table.","commit_id":"73843e506cf433dd487f56bb50ed35eeeecc5003"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"d1764f0c04d7f166d7aafd633f65765b6e39bdf3","unresolved":true,"context_lines":[{"line_number":332,"context_line":"                for allowed_address_pair in p.get(\u0027allowed_address_pairs\u0027, []):"},{"line_number":333,"context_line":"                    allowed_address_pair_net \u003d netaddr.IPNetwork("},{"line_number":334,"context_line":"                        allowed_address_pair[\u0027ip_address\u0027])"},{"line_number":335,"context_line":"                    for ip_address in allowed_address_pair_net:"},{"line_number":336,"context_line":"                        self._update_arp_entry("},{"line_number":337,"context_line":"                            str(ip_address),"},{"line_number":338,"context_line":"                            allowed_address_pair[\u0027mac_address\u0027],"}],"source_content_type":"text/x-python","patch_set":1,"id":"c177963e_0f178c49","line":335,"range":{"start_line":335,"start_character":20,"end_line":335,"end_character":62},"in_reply_to":"d55db39d_a8bd9fa9","updated":"2021-07-09 15:32:47.000000000","message":"I tested with cidr\u003d\u0027192.168.50.0/18\u0027 (16K addresses).\n\nIt took 24 secs in my env. I changed a bit \"neutron.privileged.agent.linux.ip_lib.add_neigh_entry\" to accept CIDRs instead of IPs. So, in one single privsep call, we iterate over the CIDR IPs adding the ARP register. With that chaage, my env took 13 secs to finish.","commit_id":"73843e506cf433dd487f56bb50ed35eeeecc5003"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"29ffd26e346ede6bed2261b0c3fdb61a8f5d3901","unresolved":true,"context_lines":[{"line_number":333,"context_line":"                    allowed_address_pair_net \u003d netaddr.IPNetwork("},{"line_number":334,"context_line":"                        allowed_address_pair[\u0027ip_address\u0027])"},{"line_number":335,"context_line":"                    for ip_address in allowed_address_pair_net:"},{"line_number":336,"context_line":"                        self._update_arp_entry("},{"line_number":337,"context_line":"                            str(ip_address),"},{"line_number":338,"context_line":"                            allowed_address_pair[\u0027mac_address\u0027],"},{"line_number":339,"context_line":"                            subnet_id,"}],"source_content_type":"text/x-python","patch_set":1,"id":"852e9bdc_37acc543","line":336,"range":{"start_line":336,"start_character":29,"end_line":336,"end_character":46},"updated":"2021-07-12 01:41:31.000000000","message":"Besides, so many arp entry in network namespace may need to change related Linux kernel parameters. Otherwise, the kernel will consume a lot of computing time in ARP GC [1]. Then, all routers in this agent may get influenced.\n\n[1] https://linux.die.net/man/7/arp","commit_id":"73843e506cf433dd487f56bb50ed35eeeecc5003"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"62169fee08d06b97f7b310045f1b91dfa77755c1","unresolved":true,"context_lines":[{"line_number":330,"context_line":"                                           device\u003ddevice,"},{"line_number":331,"context_line":"                                           device_exists\u003ddevice_exists)"},{"line_number":332,"context_line":"                for allowed_address_pair in p.get(\u0027allowed_address_pairs\u0027, []):"},{"line_number":333,"context_line":"                    ip_address \u003d None"},{"line_number":334,"context_line":"                    if (\u0027/\u0027 not in str(allowed_address_pair[\u0027ip_address\u0027]) or"},{"line_number":335,"context_line":"                            common_utils.is_cidr_host("},{"line_number":336,"context_line":"                                allowed_address_pair[\u0027ip_address\u0027])):"}],"source_content_type":"text/x-python","patch_set":2,"id":"a6473f06_baa1a50a","line":333,"range":{"start_line":333,"start_character":20,"end_line":333,"end_character":30},"updated":"2021-08-09 13:46:52.000000000","message":"Just a question: what about to care of big CIDRs? As commented in PS1. With big CIDRs, that will take too much time and resources to write so many ARPs entries.","commit_id":"a5e1e6155378910f47c8f2f60c9d950ed29db863"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"709ac91ab0adf4e7bc7db5b1998da61f24748d4d","unresolved":true,"context_lines":[{"line_number":330,"context_line":"                                           device\u003ddevice,"},{"line_number":331,"context_line":"                                           device_exists\u003ddevice_exists)"},{"line_number":332,"context_line":"                for allowed_address_pair in p.get(\u0027allowed_address_pairs\u0027, []):"},{"line_number":333,"context_line":"                    ip_address \u003d None"},{"line_number":334,"context_line":"                    if (\u0027/\u0027 not in str(allowed_address_pair[\u0027ip_address\u0027]) or"},{"line_number":335,"context_line":"                            common_utils.is_cidr_host("},{"line_number":336,"context_line":"                                allowed_address_pair[\u0027ip_address\u0027])):"}],"source_content_type":"text/x-python","patch_set":2,"id":"3b104884_1c958514","line":333,"range":{"start_line":333,"start_character":20,"end_line":333,"end_character":30},"in_reply_to":"80b5582b_f3486c48","updated":"2021-09-07 10:34:20.000000000","message":"I think I was thinking in PS1, not this version. I agree with this and the follow up patch documenting this limitation: only /32 allowed address pairs will be populated in the ARP table of the local routers. It is understandable.","commit_id":"a5e1e6155378910f47c8f2f60c9d950ed29db863"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"abea09700775346cf8bf70de8af210e28247f4bf","unresolved":true,"context_lines":[{"line_number":330,"context_line":"                                           device\u003ddevice,"},{"line_number":331,"context_line":"                                           device_exists\u003ddevice_exists)"},{"line_number":332,"context_line":"                for allowed_address_pair in p.get(\u0027allowed_address_pairs\u0027, []):"},{"line_number":333,"context_line":"                    ip_address \u003d None"},{"line_number":334,"context_line":"                    if (\u0027/\u0027 not in str(allowed_address_pair[\u0027ip_address\u0027]) or"},{"line_number":335,"context_line":"                            common_utils.is_cidr_host("},{"line_number":336,"context_line":"                                allowed_address_pair[\u0027ip_address\u0027])):"}],"source_content_type":"text/x-python","patch_set":2,"id":"80b5582b_f3486c48","line":333,"range":{"start_line":333,"start_character":20,"end_line":333,"end_character":30},"in_reply_to":"9aba838c_a9ce348f","updated":"2021-08-26 19:09:12.000000000","message":"It\u0027s exactly how Pavlo wrote - only /32 IPs are added to the arp table in router\u0027s namespace. I think that this will not have any performance impact and will just avoid ugly failure in case when bigger cidr is added to port\u0027s allowed_address_pairs.\n@Pavlo - done","commit_id":"a5e1e6155378910f47c8f2f60c9d950ed29db863"},{"author":{"_account_id":9542,"name":"Pavlo Shchelokovskyy","email":"pshchelokovskyy@mirantis.com","username":"pshchelo"},"change_message_id":"0f2eb282ba68307ed0d4d7647f3863f2adbdb787","unresolved":true,"context_lines":[{"line_number":330,"context_line":"                                           device\u003ddevice,"},{"line_number":331,"context_line":"                                           device_exists\u003ddevice_exists)"},{"line_number":332,"context_line":"                for allowed_address_pair in p.get(\u0027allowed_address_pairs\u0027, []):"},{"line_number":333,"context_line":"                    ip_address \u003d None"},{"line_number":334,"context_line":"                    if (\u0027/\u0027 not in str(allowed_address_pair[\u0027ip_address\u0027]) or"},{"line_number":335,"context_line":"                            common_utils.is_cidr_host("},{"line_number":336,"context_line":"                                allowed_address_pair[\u0027ip_address\u0027])):"}],"source_content_type":"text/x-python","patch_set":2,"id":"9aba838c_a9ce348f","line":333,"range":{"start_line":333,"start_character":20,"end_line":333,"end_character":30},"in_reply_to":"a6473f06_baa1a50a","updated":"2021-08-23 10:55:37.000000000","message":"Rodolfo, in this version of patch, only the single IP or single address CIDRs will be added as arp entries, so size of a CIDR is not a concern\nAFAIU we will update docs etc to specifically state that only such CIDRs are supported for allowed address pairs in DVR\n\nSlawek, also nit - this line is not needed any more","commit_id":"a5e1e6155378910f47c8f2f60c9d950ed29db863"}]}