)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"working for Ericsson, UTC+1 (+DST)"},"change_message_id":"48a7d4e08dc146c3a6fd4e65e54a7b114eedad15","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":17,"id":"e5380343_5435bc30","updated":"2021-10-14 14:30:38.000000000","message":"I just started reviewing these patches. Will continue next Monday. I also want to pull these patches into my devstack and try them there.","commit_id":"9fe3760e1d628d38131edccefcf022e3dba9c211"},{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"working for Ericsson, UTC+1 (+DST)"},"change_message_id":"8334362b05c33bfe8f020ad3bdfe30b6ce08e57b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":18,"id":"bda86331_4d7c7ae4","updated":"2021-10-27 14:16:19.000000000","message":"An snat enabled router is accepted, but again if I remember correctly it should be rejected:\n\n openstack address scope create scope0\n openstack subnet pool create pool0 --address-scope scope0 --pool-prefix 10.0.0.0/8 --default-prefix-length 24\n \n openstack network create net0 --provider-network-type vlan --provider-physical-network physnet0 --provider-segment 200 --external\n openstack subnet create subnet0 --network net0 --subnet-pool pool0 --subnet-range 10.0.0.0/24 --gateway 10.0.0.1 --no-dhcp\n \n openstack network create net1\n openstack subnet create subnet1 --network net1 --subnet-pool pool0 --subnet-range 10.0.1.0/24\n \n openstack router create router0\n openstack router set --enable-snat --external-gateway net0 router0\n openstack router add subnet router0 subnet1\n \n openstack bgp speaker create speaker0 --no-advertise-floating-ip-host-routes --local-as 1\n openstack bgp peer create peer0 --peer 10.0.1.10 --remote-as 2\n \n openstack bgp speaker router association create speaker0-router0 speaker0 router0\n openstack bgp speaker peer association create speaker0-peer0 speaker0 peer0\n \n openstack bgp speaker router association show speaker0-router0 speaker0\n openstack bgp speaker peer association show speaker0-peer0 speaker0\n","commit_id":"7561a8327344b12079694456866f77ce9061832d"},{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"working for Ericsson, UTC+1 (+DST)"},"change_message_id":"4c303de928694a816387a4c72cd7ebd3184784e9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":18,"id":"de30908f_084b6963","updated":"2021-10-28 14:16:10.000000000","message":"Continued with the tests, some more of my findings:\n\n\"openstack bgp speaker list advertised routes\" does not seem to list anything even when the speaker actually has routes to advertise.","commit_id":"7561a8327344b12079694456866f77ce9061832d"},{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"working for Ericsson, UTC+1 (+DST)"},"change_message_id":"94f1b01fde7fb8ea16730d00cd266ba6168bd60f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":18,"id":"cfe76dc3_9dc59eff","updated":"2021-10-27 14:04:16.000000000","message":"I built a devstack environment for the review of this patch series. For later use let me first paste how the environment was created:\n\nlocal.conf\n[[local|localrc]]\nenable_plugin neutron-dynamic-routing \"$DEV_GIT_BASE/openstack/neutron-dynamic-routing/.git\" \"master\"\nENABLED_SERVICES+\u003d,q-dr,q-dr-agent\n\npulled all bgpaas code into my devstack\n\nneutron-lib\na497ec7 (HEAD -\u003e bgpaas) Addition of status and name field to bgp_associations API\nf81ee6f (opendev/master, master) Merge \"BFD validator\"\n\nneutron-dynamic-routing\n597dc43 (HEAD -\u003e bgpaas) Updating status, name field and handling reboot operation\nfb38971 BGP speaker route handling\n1b4e55b BGPaaS server side implementation\nc7913ab Support peer association to BGP speaker\n37af997 Support router association to BGP speaker\nbe4bf54 (opendev/master, master) Merge \"Drop dsvm-functional tox env and related files\"\n\npython-neutronclient\nd2a9efe (HEAD -\u003e bgpaas) Add neutron and osc commands for bgp speaker peer association\n24c09b1 Add neutron and osc commands for bgp speaker router associationw\n792ad11 (opendev/master, master) Add Python3 yoga unit tests\n\nre-installed python packages: neutron-lib python-neutronclient neutron-dynamic-routing\n\nupgraded db schema\nsudo neutron-db-manage --config-file /etc/neutron/neutron.conf --config-file /etc/neutron/plugins/ml2/ml2_conf.ini upgrade heads\n\nloaded the service plugin\n\n/etc/neutron/neutron.conf\n[DEFAULT]\nservice_plugins \u003d ...,neutron_dynamic_routing.services.bgp.bgp_plugin.BgpPlugin\n\nloaded the l3 agent extension\n\n/etc/neutron/l3_agent.ini\n[agent]\nextensions \u003d ...,bgp-ext\n\nrestarted devstack services: q-svc q-l3\n\nran the following commands\n\nopenstack address scope create scope0\nopenstack subnet pool create pool0 --address-scope scope0 --pool-prefix 10.0.0.0/8 --default-prefix-length 24\n\nopenstack network create net0 --provider-network-type vlan --provider-physical-network physnet0 --provider-segment 200 --external\nopenstack subnet create subnet0 --network net0 --subnet-pool pool0 --subnet-range 10.0.0.0/24 --gateway 10.0.0.1 --no-dhcp\n\nopenstack network create net1\nopenstack subnet create subnet1 --network net1 --subnet-pool pool0 --subnet-range 10.0.1.0/24\n\nopenstack router create router0\nopenstack router set --disable-snat --external-gateway net0 router0\n\nopenstack router add subnet router0 subnet1\n\nopenstack bgp speaker create speaker0 --no-advertise-floating-ip-host-routes --advertise-tenant-networks --local-as 1\nopenstack bgp peer create peer0 --peer 10.0.1.10 --remote-as 2\n\nopenstack image create --disk-format qcow2 --public --file ~/ubuntu-20.04-server-cloudimg-amd64-root-password-quagga-installed.img u2004i\n\nopenstack server create --image u2004i --flavor ds512M --nic net-id\u003dnet1,v4-fixed-ip\u003d10.0.1.10 vm0 --wait\nsudo virsh console \"$( openstack server show vm0 -f value -c OS-EXT-SRV-ATTR:instance_name )\"\n\nvim /etc/quagga/bgpd.conf\nlog file /var/log/quagga/bgpd.log debugging\nrouter bgp 2\n bgp router-id 10.0.1.10\n network 172.16.0.0/12\n neighbor 10.0.1.1 remote-as 1\n neighbor 10.0.1.1 passive\n\nsystemctl restart bgpd\n\nopenstack bgp speaker router association create speaker0-router0 speaker0 router0\nopenstack bgp speaker peer association create speaker0-peer0 speaker0 peer0\n\nopenstack bgp speaker router association show speaker0-router0 speaker0\nopenstack bgp speaker peer association show speaker0-peer0 speaker0\n                                                                                                                                                                                           ","commit_id":"7561a8327344b12079694456866f77ce9061832d"},{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"working for Ericsson, UTC+1 (+DST)"},"change_message_id":"4c00e81d5946426786036ead748476289470e762","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":18,"id":"29bdda75_7b456180","updated":"2021-10-28 15:00:59.000000000","message":"I tried to put together an environment in which the peering happens over ipv6 (and v6 only, however out of curiosity in quagga I also added a v4 prefix to announce), but so far I could not succeed in that. This is what I did:\n\n openstack address scope create scope0 --ip-version 6\n openstack subnet pool create pool0 --address-scope scope0 --pool-prefix fc00::/56 --default-prefix-length 64\n \n openstack network create net0 --provider-network-type vlan --provider-physical-network physnet0 --provider-segment 200 --external\n openstack subnet create subnet0 --network net0 --subnet-pool pool0 --subnet-range fc00:0:0:0::/64 --gateway fc00:0:0:0:: --no-dhcp --ip-version 6 --ipv6-ra-mode dhcpv6-stateless --ipv6-address-mode dhcpv6-stateless\n \n openstack network create net1\n openstack subnet create subnet1 --network net1 --subnet-pool pool0 --subnet-range fc00:0:0:1::/64 --ip-version 6 --ipv6-ra-mode dhcpv6-stateless --ipv6-address-mode dhcpv6-stateless\n \n openstack router create router0\n openstack router set --disable-snat --external-gateway net0 router0\n \n openstack router add subnet router0 subnet1\n \n openstack bgp speaker create speaker0 --no-advertise-floating-ip-host-routes --advertise-tenant-networks --local-as 1 --ip-version 6\n \n openstack server create --image u2004c --flavor ds512M --nic net-id\u003dnet1 vm0 --wait\n sudo virsh console \"$( openstack server show vm0 -f value -c OS-EXT-SRV-ATTR:instance_name )\"\n \n vim /etc/quagga/bgpd.conf\n log file /var/log/quagga/bgpd.log debugging\n router bgp 2\n  bgp router-id 10.0.1.10\n  network 172.16.2.0/24\n  address-family ipv6\n   network fc00:2::/64\n  neighbor fc00:0:0:0:: remote-as 1\n  neighbor fc00:0:0:0:: passive\n \n systemctl restart bgpd\n \n openstack server show vm0 -f yaml -c addresses\n openstack bgp peer create peer0 --peer fc00::1:f816:3eff:feca:3f1d --remote-as 2\n \n openstack bgp speaker router association create speaker0-router0 speaker0 router0\n openstack bgp speaker peer association create speaker0-peer0 speaker0 peer0\n \n openstack bgp speaker router association show speaker0-router0 speaker0\n openstack bgp speaker peer association show speaker0-peer0 speaker0\n\nIt seems to me that a peering connection was never established. Quagga listens as expected:\n\n # ip a show dev ens2\n 2: ens2: \u003cBROADCAST,MULTICAST,UP,LOWER_UP\u003e mtu 1450 qdisc fq_codel state UP group default qlen 1000\n     link/ether fa:16:3e:ca:3f:1d brd ff:ff:ff:ff:ff:ff\n     inet6 fc00::1:f816:3eff:feca:3f1d/64 scope global dynamic mngtmpaddr noprefixroute \n        valid_lft 86361sec preferred_lft 14361sec\n     inet6 fe80::f816:3eff:feca:3f1d/64 scope link \n        valid_lft forever preferred_lft forever\n # ss -ntlp | egrep -w 179\n LISTEN    0         3                  0.0.0.0:179              0.0.0.0:*        users:((\"bgpd\",pid\u003d1223,fd\u003d7))                                                 \n LISTEN    0         3                     [::]:179                 [::]:*        users:((\"bgpd\",pid\u003d1223,fd\u003d8))\n\nBut I cannot see the peering connection and vtysh also says that the connection was never established:\n\n # show ip bgp neighbors\n BGP neighbor is fc00::, remote AS 1, local AS 2, external link\n   BGP version 4, remote router ID 0.0.0.0\n   BGP state \u003d Active (passive)\n   Last read 00:55:05, hold time is 180, keepalive interval is 60 seconds\n   Message statistics:\n     Inq depth is 0\n     Outq depth is 0\n                          Sent       Rcvd\n     Opens:                  0          0\n     Notifications:          0          0\n     Updates:                0          0\n     Keepalives:             0          0\n     Route Refresh:          0          0\n     Capability:             0          0\n     Total:                  0          0\n   Minimum time between advertisement runs is 3 seconds\n \n  For address family: IPv4 Unicast\n   Community attribute sent to this neighbor(all)\n   0 accepted prefixes\n \n   Connections established 0; dropped 0\n   Last reset never\n","commit_id":"7561a8327344b12079694456866f77ce9061832d"},{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"working for Ericsson, UTC+1 (+DST)"},"change_message_id":"302896ed72c36cda0af7e75853571852f29ae85d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":18,"id":"ba709cae_b0674a44","updated":"2021-10-27 14:18:47.000000000","message":"If I create the bgp speaker with --no-advertise-tenant-networks that seems to be respected, but if create it with --advertise-tenant-networks and later update it to --no-advertise-tenant-networks that seems to be ignored and the tenant networks are still advertised.\n\n openstack bgp speaker set speaker0 --no-advertise-tenant-networks                                                                                                                                                                             ","commit_id":"7561a8327344b12079694456866f77ce9061832d"},{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"working for Ericsson, UTC+1 (+DST)"},"change_message_id":"a48ca003ac253bafd269ae2bd358c911f1ca3ff6","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":18,"id":"25e209e6_9ce81c99","updated":"2021-10-28 14:36:42.000000000","message":"If I delete a bgp speaker that still has associations (either router or peet), I get an internal server error. The rejection is correct but neutron should respond with a 4xx code (probably 409 Conflict).\n\n $ openstack bgp speaker peer association list speaker0\n +--------------------------------------+----------------------------------+--------------------------------------+--------+\n | ID                                   | Tenant ID                        | Peer                                  | Status |\n +--------------------------------------+----------------------------------+--------------------------------------+--------+\n | 0f963527-ae77-4958-b50b-b53fed83822e | 2ede084d0f084378b26594db0ae7dd32 | 2ad7f602-62b3-45aa-90dd-f268f342479e | ACTIVE |\n +--------------------------------------+----------------------------------+--------------------------------------+--------+\n $ openstack bgp speaker delete speaker0\n Request Failed: internal server error while processing your request.\n Neutron server returns request_ids: [\u0027req-c4e6932c-b906-4f97-9fcb-9a67fc4831c5\u0027]\n \n okt 28 13:32:31 devstack0 neutron-server[7827]: ERROR neutron.api.v2.resource [None req-51fe3ba2-26ed-460e-bb5e-326d0b6c9bb5 admin admin] delete failed: No details.: oslo_db.exception.DBError: (pymysql.err.IntegrityError) (1048, \"Column \u0027\nbgp_speaker_id\u0027 cannot be null\")\n okt 28 13:32:31 devstack0 neutron-server[7827]: [SQL: UPDATE bgp_speaker_peer_associations SET bgp_speaker_id\u003d%(bgp_speaker_id)s WHERE bgp_speaker_peer_associations.id \u003d %(bgp_speaker_peer_associations_id)s]\n okt 28 13:32:31 devstack0 neutron-server[7827]: [parameters: {\u0027bgp_speaker_id\u0027: None, \u0027bgp_speaker_peer_associations_id\u0027: \u00270f963527-ae77-4958-b50b-b53fed83822e\u0027}]\n \n $ openstack bgp speaker router association create speaker0-router0 speaker0 router0\n +------------------------+--------------------------------------+\n | Field                  | Value                                |\n +------------------------+--------------------------------------+\n | advertise_extra_routes | True                                 |\n | id                     | 37e45cc2-6923-4713-bc3e-e29365e1cf22 |\n | name                   | speaker0-router0                     |\n | project_id             | 2ede084d0f084378b26594db0ae7dd32     |\n | router_id              | 34b4ce8f-3e83-43bb-9e60-a3e104e1ad4f |\n | status                 | DOWN                                 |\n | tenant_id              | 2ede084d0f084378b26594db0ae7dd32     |\n +------------------------+--------------------------------------+\n $ openstack bgp speaker delete speaker0\n Request Failed: internal server error while processing your request.\n Neutron server returns request_ids: [\u0027req-2b2eabd9-5ac8-4da1-8342-9fb974596aea\u0027]                                                                                                                                                              ","commit_id":"7561a8327344b12079694456866f77ce9061832d"},{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"working for Ericsson, UTC+1 (+DST)"},"change_message_id":"7521f241023a1d92221e271a96a6f7dd96c25a56","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":18,"id":"2639a730_2fae9262","updated":"2021-10-27 14:08:10.000000000","message":"In this environemnt the bgp speaker router and peer associations went to the ACTIVE state and I was able to confirm route exchange in both directions:\n\nin quagga before and after:\n\n vm0# show ip route\n Codes: K - kernel route, C - connected, S - static, R - RIP,\n        O - OSPF, I - IS-IS, B - BGP, P - PIM, A - Babel, N - NHRP,\n        \u003e - selected route, * - FIB route\n \n K\u003e* 0.0.0.0/0 via 10.0.1.1, ens2, src 10.0.1.10\n C\u003e* 10.0.1.0/24 is directly connected, ens2\n C\u003e* 127.0.0.0/8 is directly connected, lo\n K\u003e* 169.254.169.254/32 via 10.0.1.1, ens2, src 10.0.1.10\n vm0# show ip route·\n Codes: K - kernel route, C - connected, S - static, R - RIP,\n        O - OSPF, I - IS-IS, B - BGP, P - PIM, A - Babel, N - NHRP,\n        \u003e - selected route, * - FIB route\n \n K\u003e* 0.0.0.0/0 via 10.0.1.1, ens2, src 10.0.1.10\n B   10.0.1.0/24 [20/0] via 10.0.1.1 inactive, 00:00:02\n C\u003e* 10.0.1.0/24 is directly connected, ens2\n C\u003e* 127.0.0.0/8 is directly connected, lo\n K\u003e* 169.254.169.254/32 via 10.0.1.1, ens2, src 10.0.1.10\n\nin the neutron router namespace before and after:\n\n $ sudo ip netns exec qrouter-$( openstack router show router0 -f value -c id ) ip r\n default via 10.0.0.1 dev qg-7415cff5-db proto static\n 10.0.0.0/24 dev qg-7415cff5-db proto kernel scope link src 10.0.0.111\n 10.0.1.0/24 dev qr-fd84d648-ae proto kernel scope link src 10.0.1.1\n $ sudo ip netns exec qrouter-$( openstack router show router0 -f value -c id ) ip r\n default via 10.0.0.1 dev qg-7415cff5-db proto static\n 10.0.0.0/24 dev qg-7415cff5-db proto kernel scope link src 10.0.0.111\n 10.0.1.0/24 dev qr-fd84d648-ae proto kernel scope link src 10.0.1.1\n 172.16.0.0/12 via 10.0.1.10 dev qr-fd84d648-ae proto static","commit_id":"7561a8327344b12079694456866f77ce9061832d"},{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"working for Ericsson, UTC+1 (+DST)"},"change_message_id":"aa55f204df4f81d466f0801883d802f3e6ed160c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":18,"id":"06a301d4_0aa7f9c0","updated":"2021-10-27 14:14:42.000000000","message":"Then I started testing various corner cases (in no particular order and I still have a few on my todo list). Some of these comments may be better placed on the other router and peer association patches, but at the moment I was not reading the code yet, just testing the high level behaviors. So if you don\u0027t mind I will post these comments on this patch.\n\nNetworks not coming from an address scope are accepted, but IIRC they should not be (maybe the bgp speaker router association should reject this case):\n\n openstack network create net0 --provider-network-type vlan --provider-physical-network physnet0 --provider-segment 200 --external\n openstack subnet create subnet0 --network net0 --subnet-range 10.0.0.0/24 --gateway 10.0.0.1 --no-dhcp\n \n openstack network create net1\n openstack subnet create subnet1 --network net1 --subnet-range 10.0.1.0/24\n \n openstack router create router0\n openstack router set --disable-snat --external-gateway net0 router0\n openstack router add subnet router0 subnet1\n \n openstack bgp speaker create speaker0 --no-advertise-floating-ip-host-routes --local-as 1\n openstack bgp peer create peer0 --peer 10.0.1.10 --remote-as 2\n \n openstack bgp speaker router association create speaker0-router0 speaker0 router0\n openstack bgp speaker peer association create speaker0-peer0 speaker0 peer0\n \n openstack bgp speaker router association show speaker0-router0 speaker0\n openstack bgp speaker peer association show speaker0-peer0 speaker0\n","commit_id":"7561a8327344b12079694456866f77ce9061832d"},{"author":{"_account_id":33273,"name":"Manu B","display_name":"Manu B","email":"manu.b@est.tech","username":"eceghkl"},"change_message_id":"3ba17301263a952f6272f5a6338345194dd0369b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":18,"id":"416529ad_e1f91455","in_reply_to":"06a301d4_0aa7f9c0","updated":"2021-11-09 05:50:39.000000000","message":"The routes which are not part of address scopes will not be advertised. But the association is accepted. This is the approach which is currently used.\n\nThe same approach is followed for network-attached speakers as well. We can add network to bgp speaker but routes which are not part of address scope will not be advertised.\n\nPlease let me know if this is fine to be followed","commit_id":"7561a8327344b12079694456866f77ce9061832d"},{"author":{"_account_id":33273,"name":"Manu B","display_name":"Manu B","email":"manu.b@est.tech","username":"eceghkl"},"change_message_id":"3ba17301263a952f6272f5a6338345194dd0369b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":18,"id":"cde64044_a61639b2","in_reply_to":"25e209e6_9ce81c99","updated":"2021-11-09 05:50:39.000000000","message":"Will raise fix in next patch.\nAlso, fixed similar issue with bgp peer.","commit_id":"7561a8327344b12079694456866f77ce9061832d"},{"author":{"_account_id":33273,"name":"Manu B","display_name":"Manu B","email":"manu.b@est.tech","username":"eceghkl"},"change_message_id":"3ba17301263a952f6272f5a6338345194dd0369b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":18,"id":"4915d00f_f65603f6","in_reply_to":"ba709cae_b0674a44","updated":"2021-11-09 05:50:39.000000000","message":"advertise-tenant-networks wasn\u0027t honored in the router association code.\nI have added the below check while retrieving routes.\nBgpSpeaker.advertise_tenant_networks \u003d\u003d sa.sql.true(),\nThis fixed the issue if router is associated after speaker creation(with --no-advertise-tenant-networks).\n\nBut updating BGP speaker later, has to be fixed. This issue seems to be present for network attached speakers as well. Is it better to fix both of them together in a different review?","commit_id":"7561a8327344b12079694456866f77ce9061832d"},{"author":{"_account_id":33273,"name":"Manu B","display_name":"Manu B","email":"manu.b@est.tech","username":"eceghkl"},"change_message_id":"3ba17301263a952f6272f5a6338345194dd0369b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":18,"id":"3fa27a56_c90a55f6","in_reply_to":"bda86331_4d7c7ae4","updated":"2021-11-09 05:50:39.000000000","message":"Thanks, will be addressed in the next patchset","commit_id":"7561a8327344b12079694456866f77ce9061832d"},{"author":{"_account_id":33273,"name":"Manu B","display_name":"Manu B","email":"manu.b@est.tech","username":"eceghkl"},"change_message_id":"3ba17301263a952f6272f5a6338345194dd0369b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":18,"id":"738c7b65_18ef59af","in_reply_to":"cfe76dc3_9dc59eff","updated":"2021-11-09 05:50:39.000000000","message":"Thanks for checking this in detail","commit_id":"7561a8327344b12079694456866f77ce9061832d"},{"author":{"_account_id":33273,"name":"Manu B","display_name":"Manu B","email":"manu.b@est.tech","username":"eceghkl"},"change_message_id":"3ba17301263a952f6272f5a6338345194dd0369b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":19,"id":"ee30ace7_0d5f796e","updated":"2021-11-09 05:50:39.000000000","message":"Thanks for the review comments.\nHave address majority of them.\nWill address the pending comments in the next patch","commit_id":"d2dcf090a0465534b31ef80a58fce94f06121361"}],"neutron_dynamic_routing/privileged/driver.py":[{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"working for Ericsson, UTC+1 (+DST)"},"change_message_id":"48a7d4e08dc146c3a6fd4e65e54a7b114eedad15","unresolved":true,"context_lines":[{"line_number":29,"context_line":"from neutron_dynamic_routing.services.bgp.agent.driver import utils"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"eventlet.monkey_patch()"},{"line_number":32,"context_line":"libc \u003d cdll.LoadLibrary(\u0027libc.so.6\u0027)"},{"line_number":33,"context_line":"_setns \u003d libc.setns"},{"line_number":34,"context_line":"CLONE_NEWNET \u003d 0x40000000"},{"line_number":35,"context_line":"LOG \u003d logging.getLogger(__name__)"}],"source_content_type":"text/x-python","patch_set":17,"id":"fb9966a5_ab6194a1","line":32,"updated":"2021-10-14 14:30:38.000000000","message":"IIRC cdll and eventlet together can cause some nasty bugs, does the fix for the following bug apply here?\nhttps://bugs.launchpad.net/neutron/+bug/1870352","commit_id":"9fe3760e1d628d38131edccefcf022e3dba9c211"},{"author":{"_account_id":33273,"name":"Manu B","display_name":"Manu B","email":"manu.b@est.tech","username":"eceghkl"},"change_message_id":"3ba17301263a952f6272f5a6338345194dd0369b","unresolved":false,"context_lines":[{"line_number":29,"context_line":"from neutron_dynamic_routing.services.bgp.agent.driver import utils"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"eventlet.monkey_patch()"},{"line_number":32,"context_line":"libc \u003d cdll.LoadLibrary(\u0027libc.so.6\u0027)"},{"line_number":33,"context_line":"_setns \u003d libc.setns"},{"line_number":34,"context_line":"CLONE_NEWNET \u003d 0x40000000"},{"line_number":35,"context_line":"LOG \u003d logging.getLogger(__name__)"}],"source_content_type":"text/x-python","patch_set":17,"id":"4eea3a2c_25b3ff2a","line":32,"in_reply_to":"fb9966a5_ab6194a1","updated":"2021-11-09 05:50:39.000000000","message":"Thanks. will change to pydll","commit_id":"9fe3760e1d628d38131edccefcf022e3dba9c211"},{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"working for Ericsson, UTC+1 (+DST)"},"change_message_id":"48a7d4e08dc146c3a6fd4e65e54a7b114eedad15","unresolved":true,"context_lines":[{"line_number":65,"context_line":"        endpoint \u003d socket.create_connection((RPC_HOST, RPC_PORT))"},{"line_number":66,"context_line":"        client \u003d rpc.Client(endpoint)"},{"line_number":67,"context_line":"        client.call(\u0027core.stop\u0027, [])"},{"line_number":68,"context_line":"        process \u003d PROCESS_CACHE.get_bgp_speaker_process(bgp_speaker_id)"},{"line_number":69,"context_line":"        process.terminate()"},{"line_number":70,"context_line":""},{"line_number":71,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"b3fb5342_bd391c5c","line":68,"range":{"start_line":68,"start_character":32,"end_line":68,"end_character":55},"updated":"2021-10-14 14:30:38.000000000","message":"Is this intentionally get and not pop?","commit_id":"9fe3760e1d628d38131edccefcf022e3dba9c211"},{"author":{"_account_id":33273,"name":"Manu B","display_name":"Manu B","email":"manu.b@est.tech","username":"eceghkl"},"change_message_id":"3ba17301263a952f6272f5a6338345194dd0369b","unresolved":false,"context_lines":[{"line_number":65,"context_line":"        endpoint \u003d socket.create_connection((RPC_HOST, RPC_PORT))"},{"line_number":66,"context_line":"        client \u003d rpc.Client(endpoint)"},{"line_number":67,"context_line":"        client.call(\u0027core.stop\u0027, [])"},{"line_number":68,"context_line":"        process \u003d PROCESS_CACHE.get_bgp_speaker_process(bgp_speaker_id)"},{"line_number":69,"context_line":"        process.terminate()"},{"line_number":70,"context_line":""},{"line_number":71,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"d6cc8ec6_f0ad490f","line":68,"range":{"start_line":68,"start_character":32,"end_line":68,"end_character":55},"in_reply_to":"b3fb5342_bd391c5c","updated":"2021-11-09 05:50:39.000000000","message":"will change to pop action","commit_id":"9fe3760e1d628d38131edccefcf022e3dba9c211"},{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"working for Ericsson, UTC+1 (+DST)"},"change_message_id":"48a7d4e08dc146c3a6fd4e65e54a7b114eedad15","unresolved":true,"context_lines":[{"line_number":118,"context_line":"                         peer_down_handler\u003dself.bgp_peer_down_event,"},{"line_number":119,"context_line":"                         peer_up_handler\u003dself.bgp_peer_up_event)"},{"line_number":120,"context_line":""},{"line_number":121,"context_line":"        hub.spawn(net_ctrl.NET_CONTROLLER.start, *[],"},{"line_number":122,"context_line":"                  **{net_ctrl.NC_RPC_BIND_IP: \u00270.0.0.0\u0027,"},{"line_number":123,"context_line":"                     net_ctrl.NC_RPC_BIND_PORT: RPC_PORT}).wait()"},{"line_number":124,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"73c98580_6eb9fc19","line":121,"range":{"start_line":121,"start_character":49,"end_line":121,"end_character":52},"updated":"2021-10-14 14:30:38.000000000","message":"Does this have any function?","commit_id":"9fe3760e1d628d38131edccefcf022e3dba9c211"},{"author":{"_account_id":33273,"name":"Manu B","display_name":"Manu B","email":"manu.b@est.tech","username":"eceghkl"},"change_message_id":"3ba17301263a952f6272f5a6338345194dd0369b","unresolved":false,"context_lines":[{"line_number":118,"context_line":"                         peer_down_handler\u003dself.bgp_peer_down_event,"},{"line_number":119,"context_line":"                         peer_up_handler\u003dself.bgp_peer_up_event)"},{"line_number":120,"context_line":""},{"line_number":121,"context_line":"        hub.spawn(net_ctrl.NET_CONTROLLER.start, *[],"},{"line_number":122,"context_line":"                  **{net_ctrl.NC_RPC_BIND_IP: \u00270.0.0.0\u0027,"},{"line_number":123,"context_line":"                     net_ctrl.NC_RPC_BIND_PORT: RPC_PORT}).wait()"},{"line_number":124,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"25e3c095_2586bf6f","line":121,"range":{"start_line":121,"start_character":49,"end_line":121,"end_character":52},"in_reply_to":"73c98580_6eb9fc19","updated":"2021-11-09 05:50:39.000000000","message":"I think it is not required. Thanks, will remove it","commit_id":"9fe3760e1d628d38131edccefcf022e3dba9c211"}],"neutron_dynamic_routing/privileged/utils.py":[{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"working for Ericsson, UTC+1 (+DST)"},"change_message_id":"48a7d4e08dc146c3a6fd4e65e54a7b114eedad15","unresolved":true,"context_lines":[{"line_number":18,"context_line":"    process. This class holds bgp speaker id as the key and the process object"},{"line_number":19,"context_line":"    as the value."},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"    Version history:"},{"line_number":22,"context_line":"        1.0 - Initial version for caching multiple BGP process queue"},{"line_number":23,"context_line":"        information."},{"line_number":24,"context_line":"    \"\"\""},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"    def __init__(self):"}],"source_content_type":"text/x-python","patch_set":17,"id":"8d3724b9_af30b6da","line":23,"range":{"start_line":21,"start_character":0,"end_line":23,"end_character":20},"updated":"2021-10-14 14:30:38.000000000","message":"Classes like this may not need versioning I believe.","commit_id":"9fe3760e1d628d38131edccefcf022e3dba9c211"},{"author":{"_account_id":33273,"name":"Manu B","display_name":"Manu B","email":"manu.b@est.tech","username":"eceghkl"},"change_message_id":"3ba17301263a952f6272f5a6338345194dd0369b","unresolved":false,"context_lines":[{"line_number":18,"context_line":"    process. This class holds bgp speaker id as the key and the process object"},{"line_number":19,"context_line":"    as the value."},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"    Version history:"},{"line_number":22,"context_line":"        1.0 - Initial version for caching multiple BGP process queue"},{"line_number":23,"context_line":"        information."},{"line_number":24,"context_line":"    \"\"\""},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"    def __init__(self):"}],"source_content_type":"text/x-python","patch_set":17,"id":"6aedc801_d0966a8b","line":23,"range":{"start_line":21,"start_character":0,"end_line":23,"end_character":20},"in_reply_to":"8d3724b9_af30b6da","updated":"2021-11-09 05:50:39.000000000","message":"Done","commit_id":"9fe3760e1d628d38131edccefcf022e3dba9c211"}],"neutron_dynamic_routing/services/bgp/agent/l3/bgp_extension.py":[{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"working for Ericsson, UTC+1 (+DST)"},"change_message_id":"48a7d4e08dc146c3a6fd4e65e54a7b114eedad15","unresolved":true,"context_lines":[{"line_number":83,"context_line":"                                                      assoc.bgp_speaker_id)"},{"line_number":84,"context_line":"            if event_type \u003d\u003d rpc_events.CREATED:"},{"line_number":85,"context_line":"                priv_driver.add_bgp_speaker(assoc.bgp_speaker_id,"},{"line_number":86,"context_line":"                                            sp[\u0027local_as\u0027], \u00271.1.1.1\u0027,"},{"line_number":87,"context_line":"                                            router_info.ns_name,"},{"line_number":88,"context_line":"                                            sp[\u0027ip_version\u0027])"},{"line_number":89,"context_line":"            elif event_type \u003d\u003d rpc_events.DELETED:"}],"source_content_type":"text/x-python","patch_set":17,"id":"6c710298_f01b981c","line":86,"range":{"start_line":86,"start_character":61,"end_line":86,"end_character":68},"updated":"2021-10-14 14:30:38.000000000","message":"I guess this is WIP.","commit_id":"9fe3760e1d628d38131edccefcf022e3dba9c211"},{"author":{"_account_id":33273,"name":"Manu B","display_name":"Manu B","email":"manu.b@est.tech","username":"eceghkl"},"change_message_id":"3ba17301263a952f6272f5a6338345194dd0369b","unresolved":true,"context_lines":[{"line_number":83,"context_line":"                                                      assoc.bgp_speaker_id)"},{"line_number":84,"context_line":"            if event_type \u003d\u003d rpc_events.CREATED:"},{"line_number":85,"context_line":"                priv_driver.add_bgp_speaker(assoc.bgp_speaker_id,"},{"line_number":86,"context_line":"                                            sp[\u0027local_as\u0027], \u00271.1.1.1\u0027,"},{"line_number":87,"context_line":"                                            router_info.ns_name,"},{"line_number":88,"context_line":"                                            sp[\u0027ip_version\u0027])"},{"line_number":89,"context_line":"            elif event_type \u003d\u003d rpc_events.DELETED:"}],"source_content_type":"text/x-python","patch_set":17,"id":"08c3ea60_5c318d01","line":86,"range":{"start_line":86,"start_character":61,"end_line":86,"end_character":68},"in_reply_to":"6c710298_f01b981c","updated":"2021-11-09 05:50:39.000000000","message":"Can I put 127.0.0.1 as BGP-router-id for all of the BGP speakers spawned inside the router? Or is it better to be taken from user.?\n\nFor network-attached speakers, BGP-router-id is obtained from /etc/neutron/bgp_dragent.ini. But in the current scenario, getting it from config files might be difficult since there will be multiple routers/bgp-speakers.\n\nPlease let me know your views.\nFor now, I just changed it to 127.0.0.1 in patch 18","commit_id":"9fe3760e1d628d38131edccefcf022e3dba9c211"}]}