)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"805319a74e8e87be0dba34684ce45c36e6aedbe3","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     lzklibj \u003clzklibj@cn.ibm.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2015-11-22 14:49:42 +0800"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"remove FIP ip from centralized router device"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Current code will try to adding floating IP address onto"},{"line_number":10,"context_line":"external gateway device in centralized router namespace."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":10,"id":"ba8a016a_bd015a08","line":7,"updated":"2015-11-24 05:12:26.000000000","message":"Maybe more suitable with \"Remove floatingip address from centralized router device.\"","commit_id":"af71c5f18c92f810f650f37a23278b9248f77945"},{"author":{"_account_id":13667,"name":"ZongKai LI","email":"onionpiece@163.com","username":"lzklibj"},"change_message_id":"a42c5e7525e36d9d947629d3b896cdf96be51759","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     lzklibj \u003clzklibj@cn.ibm.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2015-11-22 14:49:42 +0800"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"remove FIP ip from centralized router device"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Current code will try to adding floating IP address onto"},{"line_number":10,"context_line":"external gateway device in centralized router namespace."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":10,"id":"ba8a016a_667a7af3","line":7,"in_reply_to":"ba8a016a_0757ef02","updated":"2015-11-24 08:49:41.000000000","message":"Done","commit_id":"af71c5f18c92f810f650f37a23278b9248f77945"},{"author":{"_account_id":13667,"name":"ZongKai LI","email":"onionpiece@163.com","username":"lzklibj"},"change_message_id":"8dd546c57fff5b8b2dee95cd195cfe661f4ff085","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     lzklibj \u003clzklibj@cn.ibm.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2015-11-22 14:49:42 +0800"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"remove FIP ip from centralized router device"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Current code will try to adding floating IP address onto"},{"line_number":10,"context_line":"external gateway device in centralized router namespace."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":10,"id":"ba8a016a_0757ef02","line":7,"in_reply_to":"ba8a016a_bd015a08","updated":"2015-11-24 06:44:51.000000000","message":"OK, I will do it later.","commit_id":"af71c5f18c92f810f650f37a23278b9248f77945"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"805319a74e8e87be0dba34684ce45c36e6aedbe3","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"remove FIP ip from centralized router device"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Current code will try to adding floating IP address onto"},{"line_number":10,"context_line":"external gateway device in centralized router namespace."},{"line_number":11,"context_line":"It\u0027s a convenient way to do proxy arp for centralized router."},{"line_number":12,"context_line":"But for DVR router, specially the DVR local router, it"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":10,"id":"ba8a016a_60ffc901","line":9,"updated":"2015-11-24 05:12:26.000000000","message":"adding -\u003e add        onto -\u003e to","commit_id":"af71c5f18c92f810f650f37a23278b9248f77945"},{"author":{"_account_id":13667,"name":"ZongKai LI","email":"onionpiece@163.com","username":"lzklibj"},"change_message_id":"a42c5e7525e36d9d947629d3b896cdf96be51759","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"remove FIP ip from centralized router device"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Current code will try to adding floating IP address onto"},{"line_number":10,"context_line":"external gateway device in centralized router namespace."},{"line_number":11,"context_line":"It\u0027s a convenient way to do proxy arp for centralized router."},{"line_number":12,"context_line":"But for DVR router, specially the DVR local router, it"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":10,"id":"ba8a016a_2609d284","line":9,"in_reply_to":"ba8a016a_60ffc901","updated":"2015-11-24 08:49:41.000000000","message":"Done","commit_id":"af71c5f18c92f810f650f37a23278b9248f77945"},{"author":{"_account_id":13667,"name":"ZongKai LI","email":"onionpiece@163.com","username":"lzklibj"},"change_message_id":"7b485d294d39bfaa4b1542cbe13449485565a263","unresolved":false,"context_lines":[{"line_number":10,"context_line":"external gateway device in centralized router namespace."},{"line_number":11,"context_line":"It\u0027s a convenient way to do proxy arp for centralized router."},{"line_number":12,"context_line":"But for DVR router, specially the DVR local router, it"},{"line_number":13,"context_line":"inherits the same why to configure floating IP address, so"},{"line_number":14,"context_line":"it will cause issue in [1]."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"The solution is fixing previous SNAT iptable rules and removing"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":10,"id":"ba8a016a_e8a19b47","line":13,"updated":"2015-11-23 06:56:50.000000000","message":"s/why/way\n\nwill update this in next patch set.","commit_id":"af71c5f18c92f810f650f37a23278b9248f77945"},{"author":{"_account_id":13667,"name":"ZongKai LI","email":"onionpiece@163.com","username":"lzklibj"},"change_message_id":"a42c5e7525e36d9d947629d3b896cdf96be51759","unresolved":false,"context_lines":[{"line_number":10,"context_line":"external gateway device in centralized router namespace."},{"line_number":11,"context_line":"It\u0027s a convenient way to do proxy arp for centralized router."},{"line_number":12,"context_line":"But for DVR router, specially the DVR local router, it"},{"line_number":13,"context_line":"inherits the same why to configure floating IP address, so"},{"line_number":14,"context_line":"it will cause issue in [1]."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"The solution is fixing previous SNAT iptable rules and removing"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":10,"id":"ba8a016a_a61dc2bf","line":13,"in_reply_to":"ba8a016a_e8a19b47","updated":"2015-11-24 08:49:41.000000000","message":"Done","commit_id":"af71c5f18c92f810f650f37a23278b9248f77945"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"805319a74e8e87be0dba34684ce45c36e6aedbe3","unresolved":false,"context_lines":[{"line_number":23,"context_line":"certain devices, and adding route for floating IP address to do proxy"},{"line_number":24,"context_line":"arp."},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"Additionally, this patch also fix fixed_ip ip error"},{"line_number":27,"context_line":"for floating IP in functional test."},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"[1] a VM ping another VM on the same host with the later one\u0027s"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":10,"id":"ba8a016a_80618daa","line":26,"updated":"2015-11-24 05:12:26.000000000","message":"What exactly is the error here? Is it just changes to accommodate the patch here? Or an independent functional test issue.","commit_id":"af71c5f18c92f810f650f37a23278b9248f77945"},{"author":{"_account_id":13667,"name":"ZongKai LI","email":"onionpiece@163.com","username":"lzklibj"},"change_message_id":"8dd546c57fff5b8b2dee95cd195cfe661f4ff085","unresolved":false,"context_lines":[{"line_number":23,"context_line":"certain devices, and adding route for floating IP address to do proxy"},{"line_number":24,"context_line":"arp."},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"Additionally, this patch also fix fixed_ip ip error"},{"line_number":27,"context_line":"for floating IP in functional test."},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"[1] a VM ping another VM on the same host with the later one\u0027s"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":10,"id":"ba8a016a_87343fde","line":26,"in_reply_to":"ba8a016a_80618daa","updated":"2015-11-24 06:44:51.000000000","message":"I will update it later.\n\nI consider it is an independent error. But previous functional tests don\u0027t raise it because they will not check whether fixed_ip address of floating IP is in router interface subnet or not.","commit_id":"af71c5f18c92f810f650f37a23278b9248f77945"},{"author":{"_account_id":13667,"name":"ZongKai LI","email":"onionpiece@163.com","username":"lzklibj"},"change_message_id":"a42c5e7525e36d9d947629d3b896cdf96be51759","unresolved":false,"context_lines":[{"line_number":23,"context_line":"certain devices, and adding route for floating IP address to do proxy"},{"line_number":24,"context_line":"arp."},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"Additionally, this patch also fix fixed_ip ip error"},{"line_number":27,"context_line":"for floating IP in functional test."},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"[1] a VM ping another VM on the same host with the later one\u0027s"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":10,"id":"ba8a016a_0625ae07","line":26,"in_reply_to":"ba8a016a_87343fde","updated":"2015-11-24 08:49:41.000000000","message":"Done","commit_id":"af71c5f18c92f810f650f37a23278b9248f77945"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"0cac02f877eeaf33e9c8435962d9a84b3bff1ee9","unresolved":false,"context_lines":[{"line_number":11,"context_line":"It\u0027s a convenient way to do proxy arp for centralized router."},{"line_number":12,"context_line":"But for DVR router, specially the DVR local router, it"},{"line_number":13,"context_line":"inherits the same way to configure floating IP address, so"},{"line_number":14,"context_line":"it will cause issue in [1]."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"The solution is fixing previous SNAT iptable rules and removing"},{"line_number":17,"context_line":"floating IP addresses from router_2_fip device in DVR local router"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":11,"id":"9a8ffd7b_6f9f1260","line":14,"updated":"2015-11-24 21:28:07.000000000","message":"I think this is pretty well stated and matches my understanding.  You might clarify why this causes a problem.  Let me try to state it here:\n\nTraffic sent to a floating IP which happens to map to a fixed ip on the same network should go through the routers default SNAT.  In DVR, this means routing the traffic through the centralized SNAT part of the router.  The problem is that since the IP address is configured on the router\u0027s device, it thinks it owns the address and will not route it through.","commit_id":"3187a1d1de659fe51715ff5105fe69398d9d845d"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"0cac02f877eeaf33e9c8435962d9a84b3bff1ee9","unresolved":false,"context_lines":[{"line_number":17,"context_line":"floating IP addresses from router_2_fip device in DVR local router"},{"line_number":18,"context_line":"namespace. The needed-by patch will work on that."},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"But that will make DVR router has different behaviors on floating"},{"line_number":21,"context_line":"IP processing. Per [2], we can unify floating IP processing for DVR"},{"line_number":22,"context_line":"router and centralized router, by removing floating IP address from"},{"line_number":23,"context_line":"certain devices, and adding route for floating IP address to do proxy"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":11,"id":"9a8ffd7b_8f74b6fd","line":20,"updated":"2015-11-24 21:28:07.000000000","message":"*But that will mean the DVR router would handle floating IPs inconsistently with non-DVR routers.  Since the only reason to add the address to the router\u0027s interface is for proxyarp, we can instead configure proxyarp in all cases to be consistent.","commit_id":"3187a1d1de659fe51715ff5105fe69398d9d845d"},{"author":{"_account_id":13667,"name":"ZongKai LI","email":"onionpiece@163.com","username":"lzklibj"},"change_message_id":"f1c75dbbde5e05b508ba2df173790d1c1ed99f62","unresolved":false,"context_lines":[{"line_number":17,"context_line":"floating IP addresses from router_2_fip device in DVR local router"},{"line_number":18,"context_line":"namespace. The needed-by patch will work on that."},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"But that will make DVR router has different behaviors on floating"},{"line_number":21,"context_line":"IP processing. Per [2], we can unify floating IP processing for DVR"},{"line_number":22,"context_line":"router and centralized router, by removing floating IP address from"},{"line_number":23,"context_line":"certain devices, and adding route for floating IP address to do proxy"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":11,"id":"7a740942_19803a8a","line":20,"in_reply_to":"9a8ffd7b_8f74b6fd","updated":"2015-12-09 10:47:18.000000000","message":"Done","commit_id":"3187a1d1de659fe51715ff5105fe69398d9d845d"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"0cac02f877eeaf33e9c8435962d9a84b3bff1ee9","unresolved":false,"context_lines":[{"line_number":26,"context_line":"Additionally, this patch also fix fixed_ip IP error for floating IP,"},{"line_number":27,"context_line":"in [3]l3_test_common and functional tests. The floating IP which we"},{"line_number":28,"context_line":"are going to use in tests, its fixed_ip IP address doesn\u0027t belong to"},{"line_number":29,"context_line":"[4]router interface subnets scope."},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"[1] a VM ping another VM on the same host with the later one\u0027s"},{"line_number":32,"context_line":"floating IP address, but get replied by the later one\u0027s fixed"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":11,"id":"9a8ffd7b_ef34624b","line":29,"updated":"2015-11-24 21:28:07.000000000","message":"I don\u0027t understand this paragraph.","commit_id":"3187a1d1de659fe51715ff5105fe69398d9d845d"},{"author":{"_account_id":13667,"name":"ZongKai LI","email":"onionpiece@163.com","username":"lzklibj"},"change_message_id":"f1c75dbbde5e05b508ba2df173790d1c1ed99f62","unresolved":false,"context_lines":[{"line_number":26,"context_line":"Additionally, this patch also fix fixed_ip IP error for floating IP,"},{"line_number":27,"context_line":"in [3]l3_test_common and functional tests. The floating IP which we"},{"line_number":28,"context_line":"are going to use in tests, its fixed_ip IP address doesn\u0027t belong to"},{"line_number":29,"context_line":"[4]router interface subnets scope."},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"[1] a VM ping another VM on the same host with the later one\u0027s"},{"line_number":32,"context_line":"floating IP address, but get replied by the later one\u0027s fixed"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":11,"id":"7a740942_5961e203","line":29,"in_reply_to":"9a8ffd7b_8af1fa97","updated":"2015-12-09 10:47:18.000000000","message":"Try to explain more clear in new patch set.","commit_id":"3187a1d1de659fe51715ff5105fe69398d9d845d"},{"author":{"_account_id":13667,"name":"ZongKai LI","email":"onionpiece@163.com","username":"lzklibj"},"change_message_id":"8abdb61cbd303a579de29b1e0962040e6f1e9abb","unresolved":false,"context_lines":[{"line_number":26,"context_line":"Additionally, this patch also fix fixed_ip IP error for floating IP,"},{"line_number":27,"context_line":"in [3]l3_test_common and functional tests. The floating IP which we"},{"line_number":28,"context_line":"are going to use in tests, its fixed_ip IP address doesn\u0027t belong to"},{"line_number":29,"context_line":"[4]router interface subnets scope."},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"[1] a VM ping another VM on the same host with the later one\u0027s"},{"line_number":32,"context_line":"floating IP address, but get replied by the later one\u0027s fixed"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":11,"id":"9a8ffd7b_8af1fa97","line":29,"in_reply_to":"9a8ffd7b_ef34624b","updated":"2015-12-01 05:51:01.000000000","message":"For some unit tests and functional tests, they use prepare_router_data in l3_test_common to get a router data for test. That router data object will have a floating IP entry with \"fixed_ip_address\"\u003d\"10.0.0.1\", but the router\u0027s interface subnets(IPv4) cidrs are \u002735.4.x.0/24\u0027(x\u003d0,1,..), no one contains \u002710.0.0.1\u0027, so that\u0027s what I mean fixed_ip IP error.\n\nAnd l3_test_common is not the only place this error happened.","commit_id":"3187a1d1de659fe51715ff5105fe69398d9d845d"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"0cac02f877eeaf33e9c8435962d9a84b3bff1ee9","unresolved":false,"context_lines":[{"line_number":32,"context_line":"floating IP address, but get replied by the later one\u0027s fixed"},{"line_number":33,"context_line":"IP address. https://bugs.launchpad.net/neutron/+bug/1462154"},{"line_number":34,"context_line":"[2] https://review.openstack.org/#/c/240677/10//COMMIT_MSG"},{"line_number":35,"context_line":"[3] https://github.com/openstack/neutron/blob/master/neutron/tests/common/l3_test_common.py#L112"},{"line_number":36,"context_line":"[4] https://github.com/openstack/neutron/blob/master/neutron/tests/common/l3_test_common.py#L145"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"Co-Authored-By: stephen-ma \u003cstephen.ma@hpe.com\u003e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":11,"id":"9a8ffd7b_6f9392fb","line":35,"updated":"2015-11-24 21:28:07.000000000","message":"Please make these permalinks.  Since they reference master, they are going to change over time and the link won\u0027t make sense in the future.  Also, you might consider using a URL shortener.  But, I know URL shorteners have their own issues.","commit_id":"3187a1d1de659fe51715ff5105fe69398d9d845d"},{"author":{"_account_id":13667,"name":"ZongKai LI","email":"onionpiece@163.com","username":"lzklibj"},"change_message_id":"f1c75dbbde5e05b508ba2df173790d1c1ed99f62","unresolved":false,"context_lines":[{"line_number":32,"context_line":"floating IP address, but get replied by the later one\u0027s fixed"},{"line_number":33,"context_line":"IP address. https://bugs.launchpad.net/neutron/+bug/1462154"},{"line_number":34,"context_line":"[2] https://review.openstack.org/#/c/240677/10//COMMIT_MSG"},{"line_number":35,"context_line":"[3] https://github.com/openstack/neutron/blob/master/neutron/tests/common/l3_test_common.py#L112"},{"line_number":36,"context_line":"[4] https://github.com/openstack/neutron/blob/master/neutron/tests/common/l3_test_common.py#L145"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"Co-Authored-By: stephen-ma \u003cstephen.ma@hpe.com\u003e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":11,"id":"7a740942_543d27c2","line":35,"in_reply_to":"9a8ffd7b_6f9392fb","updated":"2015-12-09 10:47:18.000000000","message":"I chose to use shorten URL for now, I will try to explain without URL if I didn\u0027t tell my points clear.","commit_id":"3187a1d1de659fe51715ff5105fe69398d9d845d"},{"author":{"_account_id":6876,"name":"stephen-ma","email":"stephenbma43@gmail.com","username":"stephen-ma"},"change_message_id":"d9c7f2c8f28a60902edb7e181c9d0fd448271248","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     lzklibj \u003clzklibj@cn.ibm.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2015-12-09 17:59:10 +0800"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Remove floatingip address from router device"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Current code will try to add floating IP address to external gateway"},{"line_number":10,"context_line":"device in centralized router namespace. It\u0027s a convenient way to do"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":12,"id":"7a740942_3fc56f8d","line":7,"updated":"2015-12-10 22:38:49.000000000","message":"Remove Floatingip Addresses from Router Device","commit_id":"263a13ae4fc8f6ed729664a340b41ea6f8ce6370"},{"author":{"_account_id":6876,"name":"stephen-ma","email":"stephenbma43@gmail.com","username":"stephen-ma"},"change_message_id":"d9c7f2c8f28a60902edb7e181c9d0fd448271248","unresolved":false,"context_lines":[{"line_number":19,"context_line":""},{"line_number":20,"context_line":"The solution is fixing previous SNAT iptable rules and removing"},{"line_number":21,"context_line":"floating IP addresses from router_2_fip device in DVR local router"},{"line_number":22,"context_line":"namespace. The needed-by patch will work on that."},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"But that will mean the DVR router would handle floating IPs inconsistently"},{"line_number":25,"context_line":"with non-DVR routers. Since the only reason to add the address to the"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":12,"id":"7a740942_74548753","line":22,"updated":"2015-12-10 22:38:49.000000000","message":"I think it is more readable give the URL to the \"Needed-By\" patch.","commit_id":"263a13ae4fc8f6ed729664a340b41ea6f8ce6370"},{"author":{"_account_id":13667,"name":"ZongKai LI","email":"onionpiece@163.com","username":"lzklibj"},"change_message_id":"683b7d967018aa51bce51768e5c01f5216494669","unresolved":false,"context_lines":[{"line_number":19,"context_line":""},{"line_number":20,"context_line":"The solution is fixing previous SNAT iptable rules and removing"},{"line_number":21,"context_line":"floating IP addresses from router_2_fip device in DVR local router"},{"line_number":22,"context_line":"namespace. The needed-by patch will work on that."},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"But that will mean the DVR router would handle floating IPs inconsistently"},{"line_number":25,"context_line":"with non-DVR routers. Since the only reason to add the address to the"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":12,"id":"7a740942_826cdfd0","line":22,"in_reply_to":"7a740942_74548753","updated":"2015-12-14 09:03:18.000000000","message":"Done","commit_id":"263a13ae4fc8f6ed729664a340b41ea6f8ce6370"},{"author":{"_account_id":6876,"name":"stephen-ma","email":"stephenbma43@gmail.com","username":"stephen-ma"},"change_message_id":"d9c7f2c8f28a60902edb7e181c9d0fd448271248","unresolved":false,"context_lines":[{"line_number":26,"context_line":"router\u0027s interface is for proxy arp, we can instead configure arp proxy"},{"line_number":27,"context_line":"in all cases to be consistent."},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"Additionally, this patch also fix fixed_ip IP error for floating IP,"},{"line_number":30,"context_line":"in method prepare_router_data and functional tests."},{"line_number":31,"context_line":"Method prepare_router_data can give us a router data object with a"},{"line_number":32,"context_line":"default floating IP with \u002710.0.0.1\u0027[2] as fixed_ip_address, but all the"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":12,"id":"7a740942_f1bba973","line":29,"updated":"2015-12-10 22:38:49.000000000","message":"this patch also fixes fixed_ip","commit_id":"263a13ae4fc8f6ed729664a340b41ea6f8ce6370"},{"author":{"_account_id":13667,"name":"ZongKai LI","email":"onionpiece@163.com","username":"lzklibj"},"change_message_id":"683b7d967018aa51bce51768e5c01f5216494669","unresolved":false,"context_lines":[{"line_number":26,"context_line":"router\u0027s interface is for proxy arp, we can instead configure arp proxy"},{"line_number":27,"context_line":"in all cases to be consistent."},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"Additionally, this patch also fix fixed_ip IP error for floating IP,"},{"line_number":30,"context_line":"in method prepare_router_data and functional tests."},{"line_number":31,"context_line":"Method prepare_router_data can give us a router data object with a"},{"line_number":32,"context_line":"default floating IP with \u002710.0.0.1\u0027[2] as fixed_ip_address, but all the"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":12,"id":"7a740942_426b57b7","line":29,"in_reply_to":"7a740942_f1bba973","updated":"2015-12-14 09:03:18.000000000","message":"Done","commit_id":"263a13ae4fc8f6ed729664a340b41ea6f8ce6370"},{"author":{"_account_id":6876,"name":"stephen-ma","email":"stephenbma43@gmail.com","username":"stephen-ma"},"change_message_id":"d9c7f2c8f28a60902edb7e181c9d0fd448271248","unresolved":false,"context_lines":[{"line_number":31,"context_line":"Method prepare_router_data can give us a router data object with a"},{"line_number":32,"context_line":"default floating IP with \u002710.0.0.1\u0027[2] as fixed_ip_address, but all the"},{"line_number":33,"context_line":"router\u0027s IPv4 cidrs are in style \u002735.4.x.0/24\u0027[3]. That doesn\u0027t make"},{"line_number":34,"context_line":"any sense."},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"[1] a VM ping another VM on the same host with the later one\u0027s"},{"line_number":37,"context_line":"floating IP address, but get replied by the later one\u0027s fixed"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":12,"id":"7a740942_d146ad7a","line":34,"updated":"2015-12-10 22:38:49.000000000","message":"Without this change, will there be failure in the unit tests?","commit_id":"263a13ae4fc8f6ed729664a340b41ea6f8ce6370"},{"author":{"_account_id":13667,"name":"ZongKai LI","email":"onionpiece@163.com","username":"lzklibj"},"change_message_id":"683b7d967018aa51bce51768e5c01f5216494669","unresolved":false,"context_lines":[{"line_number":31,"context_line":"Method prepare_router_data can give us a router data object with a"},{"line_number":32,"context_line":"default floating IP with \u002710.0.0.1\u0027[2] as fixed_ip_address, but all the"},{"line_number":33,"context_line":"router\u0027s IPv4 cidrs are in style \u002735.4.x.0/24\u0027[3]. That doesn\u0027t make"},{"line_number":34,"context_line":"any sense."},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"[1] a VM ping another VM on the same host with the later one\u0027s"},{"line_number":37,"context_line":"floating IP address, but get replied by the later one\u0027s fixed"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":12,"id":"7a740942_22e16b2a","line":34,"in_reply_to":"7a740942_d146ad7a","updated":"2015-12-14 09:03:18.000000000","message":"Yes, without that change, this patch will fail in functional tests. I will update this in commit message.","commit_id":"263a13ae4fc8f6ed729664a340b41ea6f8ce6370"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"aef1b1cd01d3d2c7f53e3ad9e629753d5e9bc9c4","unresolved":false,"context_lines":[{"line_number":26,"context_line":"router\u0027s interface is for proxy arp, we can instead configure arp proxy"},{"line_number":27,"context_line":"in all cases to be consistent."},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"Additionally, this patch also fixes fixed_ip IP error for floating IP,"},{"line_number":30,"context_line":"in method prepare_router_data and functional tests."},{"line_number":31,"context_line":"Method prepare_router_data can give us a router data object with a"},{"line_number":32,"context_line":"default floating IP with \u002710.0.0.1\u0027[3] as fixed_ip_address, but all the"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":18,"id":"fa69d971_1d57475b","line":29,"updated":"2016-01-09 03:18:54.000000000","message":"separate this fix into a different patch and make this one depend on it.","commit_id":"a5fd7c6e249a922b3be10d3b10276f44773feaa9"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"85ac831560afdb2f5490c5eeeb144452b63438cd","unresolved":false,"context_lines":[{"line_number":44,"context_line":""},{"line_number":45,"context_line":"UpgradeImpact"},{"line_number":46,"context_line":""},{"line_number":47,"context_line":"Depends-on: Icef8cf430211c4782e86f94262e4dc889680d862"},{"line_number":48,"context_line":""},{"line_number":49,"context_line":"Co-Authored-By: stephen-ma \u003cstephen.ma@hpe.com\u003e"},{"line_number":50,"context_line":"Partial-Bug: #1462154"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":18,"id":"fa69d971_0b24845b","line":47,"updated":"2016-01-06 16:55:35.000000000","message":"Sounds like this is a non-starter from Sean\u0027s review.  What is the plan going forward?","commit_id":"a5fd7c6e249a922b3be10d3b10276f44773feaa9"},{"author":{"_account_id":6876,"name":"stephen-ma","email":"stephenbma43@gmail.com","username":"stephen-ma"},"change_message_id":"b83e3cdf47ec09df9c6a62cdc55c1b0554fd0b75","unresolved":false,"context_lines":[{"line_number":30,"context_line":"floating IP address, but get replied by the later one\u0027s fixed"},{"line_number":31,"context_line":"IP address. https://bugs.launchpad.net/neutron/+bug/1462154"},{"line_number":32,"context_line":"[2] remove floating IP address from DVR router device"},{"line_number":33,"context_line":"https://review.openstack.org/#/c/246894/"},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"UpgradeImpact"},{"line_number":36,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":20,"id":"da6ed579_db30e9ea","line":33,"updated":"2016-01-13 16:13:26.000000000","message":"This patch is now in the abandoned state.  Need to restore it.","commit_id":"4f876a6c35deff37390c62a7aa8610fa3ebb04a1"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"7544fce266b1a298eed183a03afca0bf7183f438","unresolved":false,"context_lines":[{"line_number":34,"context_line":""},{"line_number":35,"context_line":"UpgradeImpact"},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"Depends-on: Icef8cf430211c4782e86f94262e4dc889680d862"},{"line_number":38,"context_line":""},{"line_number":39,"context_line":"Co-Authored-By: stephen-ma \u003cstephen.ma@hpe.com\u003e"},{"line_number":40,"context_line":"Partial-Bug: #1462154"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":27,"id":"bae84128_b95ce87d","line":37,"range":{"start_line":37,"start_character":12,"end_line":37,"end_character":53},"updated":"2016-02-24 02:56:43.000000000","message":"This will not merge!  It is a dead review that won\u0027t go anywhere and should be abandoned.  We need another plan.  Frankly, I\u0027m not sure why it was needed in the first place.  I\u0027ve made changes before to rootwrap configuration without a problem.  We need to get to the bottom of it.","commit_id":"c1f0e471a71922c9911511a7dd5887fc4fb528d7"}],"etc/neutron/rootwrap.d/l3.filters":[{"author":{"_account_id":6876,"name":"stephen-ma","email":"stephenbma43@gmail.com","username":"stephen-ma"},"change_message_id":"15805a29ffa9cb724f71519d84ad86b5e57f86b7","unresolved":false,"context_lines":[{"line_number":28,"context_line":"ip: IpFilter, ip, root"},{"line_number":29,"context_line":"find: RegExpFilter, find, root, find, /sys/class/net, -maxdepth, 1, -type, l, -printf, %.*"},{"line_number":30,"context_line":"ip_exec: IpNetnsExecFilter, ip, root"},{"line_number":31,"context_line":"arp: CommandFilter, arp, root"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"# For ip monitor"},{"line_number":34,"context_line":"kill_ip_monitor: KillFilter, root, ip, -9"}],"source_content_type":"application/octet-stream","patch_set":20,"id":"da6ed579_812c4003","line":31,"updated":"2016-01-13 16:56:12.000000000","message":"The grenade test is failing because this definition is not found after the upgrade. So should creating this CommandFilter be done in a separate patch and then backport the arp commandfilter change to liberty?","commit_id":"4f876a6c35deff37390c62a7aa8610fa3ebb04a1"}],"neutron/agent/l3/dvr_edge_ha_router.py":[{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"0cac02f877eeaf33e9c8435962d9a84b3bff1ee9","unresolved":false,"context_lines":[{"line_number":102,"context_line":"        if self.snat_namespace:"},{"line_number":103,"context_line":"            self.snat_namespace.delete()"},{"line_number":104,"context_line":""},{"line_number":105,"context_line":"    def get_existing_fips(self):"},{"line_number":106,"context_line":"        return RouterInfo.get_existing_fips(self)"},{"line_number":107,"context_line":""},{"line_number":108,"context_line":"    def _external_gateway_added(self, ex_gw_port, interface_name,"}],"source_content_type":"text/x-python","patch_set":11,"id":"9a8ffd7b_3444fa72","line":105,"updated":"2015-11-24 21:28:07.000000000","message":"Is this rename contributing to this patch set or is it merely for cosmetic reasons?  It adds to the noise level of the patch which makes it more difficult to review.","commit_id":"3187a1d1de659fe51715ff5105fe69398d9d845d"},{"author":{"_account_id":13667,"name":"ZongKai LI","email":"onionpiece@163.com","username":"lzklibj"},"change_message_id":"8abdb61cbd303a579de29b1e0962040e6f1e9abb","unresolved":false,"context_lines":[{"line_number":102,"context_line":"        if self.snat_namespace:"},{"line_number":103,"context_line":"            self.snat_namespace.delete()"},{"line_number":104,"context_line":""},{"line_number":105,"context_line":"    def get_existing_fips(self):"},{"line_number":106,"context_line":"        return RouterInfo.get_existing_fips(self)"},{"line_number":107,"context_line":""},{"line_number":108,"context_line":"    def _external_gateway_added(self, ex_gw_port, interface_name,"}],"source_content_type":"text/x-python","patch_set":11,"id":"9a8ffd7b_37d5d7b8","line":105,"in_reply_to":"9a8ffd7b_3444fa72","updated":"2015-12-01 05:51:01.000000000","message":"Indeed, I just want to give this method a more suitable for now.\n\nThis method seems only used in process_floating_ip_addresses, and its previous version will try to get cidrs from router namespace for getting existing cidrs(containing fip cidrs), so get_router_cidrs is suitable for it explain what this method will do.\n\nBut now, we are no longer to get cidrs from router namespace, so I think name like get_existing_fips seems more suitable.","commit_id":"3187a1d1de659fe51715ff5105fe69398d9d845d"}],"neutron/agent/l3/dvr_local_router.py":[{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"0cac02f877eeaf33e9c8435962d9a84b3bff1ee9","unresolved":false,"context_lines":[{"line_number":138,"context_line":"            # when the external gateway port is removed. This will be"},{"line_number":139,"context_line":"            # initiated from the server through an RPC call."},{"line_number":140,"context_line":""},{"line_number":141,"context_line":"    def add_floating_ip(self, fip, interface_name, device):"},{"line_number":142,"context_line":"        if not self._add_fip_addr_to_device(fip, device):"},{"line_number":143,"context_line":"            return l3_constants.FLOATINGIP_STATUS_ERROR"},{"line_number":144,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"9a8ffd7b_343b9aed","side":"PARENT","line":141,"updated":"2015-11-24 21:28:07.000000000","message":"The fact that interface_name and device disappear from this method actually makes me smile.","commit_id":"19ededc702ba27f1dbfc59a896788f82acdc487b"},{"author":{"_account_id":13667,"name":"ZongKai LI","email":"onionpiece@163.com","username":"lzklibj"},"change_message_id":"8abdb61cbd303a579de29b1e0962040e6f1e9abb","unresolved":false,"context_lines":[{"line_number":138,"context_line":"            # when the external gateway port is removed. This will be"},{"line_number":139,"context_line":"            # initiated from the server through an RPC call."},{"line_number":140,"context_line":""},{"line_number":141,"context_line":"    def add_floating_ip(self, fip, interface_name, device):"},{"line_number":142,"context_line":"        if not self._add_fip_addr_to_device(fip, device):"},{"line_number":143,"context_line":"            return l3_constants.FLOATINGIP_STATUS_ERROR"},{"line_number":144,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"9a8ffd7b_17e773ad","side":"PARENT","line":141,"in_reply_to":"9a8ffd7b_343b9aed","updated":"2015-12-01 05:51:01.000000000","message":"Yep, we seems don\u0027t need device any more, but arp command will need interface_name, so I think I will need add interface_name back.","commit_id":"19ededc702ba27f1dbfc59a896788f82acdc487b"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"0cac02f877eeaf33e9c8435962d9a84b3bff1ee9","unresolved":false,"context_lines":[{"line_number":140,"context_line":""},{"line_number":141,"context_line":"    def add_floating_ip(self, fip, interface_name, device):"},{"line_number":142,"context_line":"        if not self._add_fip_addr_to_device(fip, device):"},{"line_number":143,"context_line":"            return l3_constants.FLOATINGIP_STATUS_ERROR"},{"line_number":144,"context_line":""},{"line_number":145,"context_line":"        # Special Handling for DVR - update FIP namespace"},{"line_number":146,"context_line":"        ip_cidr \u003d common_utils.ip_to_cidr(fip[\u0027floating_ip_address\u0027])"}],"source_content_type":"text/x-python","patch_set":11,"id":"9a8ffd7b_941da675","side":"PARENT","line":143,"updated":"2015-11-24 21:28:07.000000000","message":"DRAFT:  Why no longer returning this?  I need to figure it out.","commit_id":"19ededc702ba27f1dbfc59a896788f82acdc487b"},{"author":{"_account_id":13667,"name":"ZongKai LI","email":"onionpiece@163.com","username":"lzklibj"},"change_message_id":"8abdb61cbd303a579de29b1e0962040e6f1e9abb","unresolved":false,"context_lines":[{"line_number":140,"context_line":""},{"line_number":141,"context_line":"    def add_floating_ip(self, fip, interface_name, device):"},{"line_number":142,"context_line":"        if not self._add_fip_addr_to_device(fip, device):"},{"line_number":143,"context_line":"            return l3_constants.FLOATINGIP_STATUS_ERROR"},{"line_number":144,"context_line":""},{"line_number":145,"context_line":"        # Special Handling for DVR - update FIP namespace"},{"line_number":146,"context_line":"        ip_cidr \u003d common_utils.ip_to_cidr(fip[\u0027floating_ip_address\u0027])"}],"source_content_type":"text/x-python","patch_set":11,"id":"9a8ffd7b_2a724eae","side":"PARENT","line":143,"in_reply_to":"9a8ffd7b_941da675","updated":"2015-12-01 05:51:01.000000000","message":"So you mean we should still return status for add_floating_ip success and fail cases?","commit_id":"19ededc702ba27f1dbfc59a896788f82acdc487b"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"ba6476fee3d8c378196e3625b74356fbd2e70b97","unresolved":false,"context_lines":[{"line_number":143,"context_line":"        ip_cidr \u003d common_utils.ip_to_cidr(fip[\u0027floating_ip_address\u0027])"},{"line_number":144,"context_line":"        self.floating_ip_added_dist(fip, ip_cidr)"},{"line_number":145,"context_line":""},{"line_number":146,"context_line":"    def remove_floating_ip(self, ip_cidr, interface_name):"},{"line_number":147,"context_line":"        super(DvrLocalRouter, self).remove_floating_ip(ip_cidr, interface_name)"},{"line_number":148,"context_line":"        self.floating_ip_removed_dist(ip_cidr)"},{"line_number":149,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"5a710552_b3afa5e2","line":146,"updated":"2015-12-15 05:35:37.000000000","message":"If it is not cidr here, maybe the name should be changed.","commit_id":"30cfbb0f1c7812ae380bed99f5f7bad91d6ef7b5"},{"author":{"_account_id":13667,"name":"ZongKai LI","email":"onionpiece@163.com","username":"lzklibj"},"change_message_id":"f4aff6d7c0568766199213f3a6fedee180dd8de0","unresolved":false,"context_lines":[{"line_number":143,"context_line":"        ip_cidr \u003d common_utils.ip_to_cidr(fip[\u0027floating_ip_address\u0027])"},{"line_number":144,"context_line":"        self.floating_ip_added_dist(fip, ip_cidr)"},{"line_number":145,"context_line":""},{"line_number":146,"context_line":"    def remove_floating_ip(self, ip_cidr, interface_name):"},{"line_number":147,"context_line":"        super(DvrLocalRouter, self).remove_floating_ip(ip_cidr, interface_name)"},{"line_number":148,"context_line":"        self.floating_ip_removed_dist(ip_cidr)"},{"line_number":149,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"5a710552_ebef69ae","line":146,"in_reply_to":"5a710552_b3afa5e2","updated":"2015-12-15 09:30:22.000000000","message":"Yes, I should unify to fip_ip.","commit_id":"30cfbb0f1c7812ae380bed99f5f7bad91d6ef7b5"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"4f4745b50c414438d1186929d4ba7ed631dbd6c4","unresolved":false,"context_lines":[{"line_number":70,"context_line":""},{"line_number":71,"context_line":"        self.iptables_manager.apply()"},{"line_number":72,"context_line":""},{"line_number":73,"context_line":"    def floating_ip_added_dist(self, fip, fip_cidr, remap):"},{"line_number":74,"context_line":"        \"\"\"Add floating IP to FIP namespace.\"\"\""},{"line_number":75,"context_line":"        floating_ip \u003d fip[\u0027floating_ip_address\u0027]"},{"line_number":76,"context_line":"        fixed_ip \u003d fip[\u0027fixed_ip_address\u0027]"}],"source_content_type":"text/x-python","patch_set":23,"id":"9a68dd71_8cf2876e","line":73,"range":{"start_line":73,"start_character":52,"end_line":73,"end_character":57},"updated":"2016-01-26 17:08:05.000000000","message":"Could you add documentation for what remap does?  I don\u0027t understand what this special case is.","commit_id":"a716d93c9d387141700c0790b1b0bb3f7843dc62"},{"author":{"_account_id":13667,"name":"ZongKai LI","email":"onionpiece@163.com","username":"lzklibj"},"change_message_id":"eca0124319feccd06ab0fc6872d9c6d50f7ae28d","unresolved":false,"context_lines":[{"line_number":70,"context_line":""},{"line_number":71,"context_line":"        self.iptables_manager.apply()"},{"line_number":72,"context_line":""},{"line_number":73,"context_line":"    def floating_ip_added_dist(self, fip, fip_cidr, remap):"},{"line_number":74,"context_line":"        \"\"\"Add floating IP to FIP namespace.\"\"\""},{"line_number":75,"context_line":"        floating_ip \u003d fip[\u0027floating_ip_address\u0027]"},{"line_number":76,"context_line":"        fixed_ip \u003d fip[\u0027fixed_ip_address\u0027]"}],"source_content_type":"text/x-python","patch_set":23,"id":"7a5de9d1_1a2cb897","line":73,"range":{"start_line":73,"start_character":52,"end_line":73,"end_character":57},"in_reply_to":"9a68dd71_8cf2876e","updated":"2016-02-02 05:56:54.000000000","message":"Done.","commit_id":"a716d93c9d387141700c0790b1b0bb3f7843dc62"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"4f4745b50c414438d1186929d4ba7ed631dbd6c4","unresolved":false,"context_lines":[{"line_number":80,"context_line":"            self.floating_ips_dict[floating_ip] \u003d rule_pr"},{"line_number":81,"context_line":"        else:"},{"line_number":82,"context_line":"            rule_pr \u003d self.floating_ips_dict[floating_ip]"},{"line_number":83,"context_line":"            ip_rule.rule.delete(ip\u003dfloating_ip,"},{"line_number":84,"context_line":"                                table\u003ddvr_fip_ns.FIP_RT_TBL,"},{"line_number":85,"context_line":"                                priority\u003drule_pr)"},{"line_number":86,"context_line":"        ip_rule.rule.add(ip\u003dfixed_ip,"}],"source_content_type":"text/x-python","patch_set":23,"id":"9a68dd71_2c435b6e","line":83,"range":{"start_line":83,"start_character":35,"end_line":83,"end_character":46},"updated":"2016-01-26 17:08:05.000000000","message":"Why ip\u003dfloating_ip here and fixed_ip on L86?","commit_id":"a716d93c9d387141700c0790b1b0bb3f7843dc62"}],"neutron/agent/l3/ha.py":[{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"ba6476fee3d8c378196e3625b74356fbd2e70b97","unresolved":false,"context_lines":[{"line_number":182,"context_line":"    def _process_fip_arp_proxy_entry(self, batched_events):"},{"line_number":183,"context_line":"        for router_id, state in batched_events:"},{"line_number":184,"context_line":"            ri \u003d self.router_info[router_id]"},{"line_number":185,"context_line":"            ri.update_fip_arp_proxy_entry()"}],"source_content_type":"text/x-python","patch_set":13,"id":"5a710552_98e2c69e","line":185,"updated":"2015-12-15 05:35:37.000000000","message":"Why not pass \"state\" to \"update_fip_arp_proxy_entry\" this can save some effort to read ha_state in that method.","commit_id":"30cfbb0f1c7812ae380bed99f5f7bad91d6ef7b5"},{"author":{"_account_id":13667,"name":"ZongKai LI","email":"onionpiece@163.com","username":"lzklibj"},"change_message_id":"54980d4d6ef21fe4f9aef5345c09454287a0e6b1","unresolved":false,"context_lines":[{"line_number":182,"context_line":"    def _process_fip_arp_proxy_entry(self, batched_events):"},{"line_number":183,"context_line":"        for router_id, state in batched_events:"},{"line_number":184,"context_line":"            ri \u003d self.router_info[router_id]"},{"line_number":185,"context_line":"            ri.update_fip_arp_proxy_entry()"}],"source_content_type":"text/x-python","patch_set":13,"id":"5a710552_faa3b99c","line":185,"in_reply_to":"5a710552_98e2c69e","updated":"2015-12-15 09:12:16.000000000","message":"Make sense to me.","commit_id":"30cfbb0f1c7812ae380bed99f5f7bad91d6ef7b5"}],"neutron/agent/l3/ha_router.py":[{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"805319a74e8e87be0dba34684ce45c36e6aedbe3","unresolved":false,"context_lines":[{"line_number":250,"context_line":""},{"line_number":251,"context_line":"    def _update_fip_route(self, fip, action, via\u003dNone):"},{"line_number":252,"context_line":"        instance \u003d self._get_keepalived_instance()"},{"line_number":253,"context_line":"        if action \u003d\u003d \u0027add\u0027:"},{"line_number":254,"context_line":"            fip_route \u003d keepalived.KeepalivedVirtualRoute(fip, via)"},{"line_number":255,"context_line":"            instance.virtual_routes.fip_routes[fip] \u003d fip_route"},{"line_number":256,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":10,"id":"ba8a016a_40a2c54c","line":253,"updated":"2015-11-24 05:12:26.000000000","message":"nit comment: use some constant here for \u0027add\u0027 and \u0027remove\u0027.","commit_id":"af71c5f18c92f810f650f37a23278b9248f77945"},{"author":{"_account_id":13667,"name":"ZongKai LI","email":"onionpiece@163.com","username":"lzklibj"},"change_message_id":"8dd546c57fff5b8b2dee95cd195cfe661f4ff085","unresolved":false,"context_lines":[{"line_number":250,"context_line":""},{"line_number":251,"context_line":"    def _update_fip_route(self, fip, action, via\u003dNone):"},{"line_number":252,"context_line":"        instance \u003d self._get_keepalived_instance()"},{"line_number":253,"context_line":"        if action \u003d\u003d \u0027add\u0027:"},{"line_number":254,"context_line":"            fip_route \u003d keepalived.KeepalivedVirtualRoute(fip, via)"},{"line_number":255,"context_line":"            instance.virtual_routes.fip_routes[fip] \u003d fip_route"},{"line_number":256,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":10,"id":"ba8a016a_514a9584","line":253,"in_reply_to":"ba8a016a_40a2c54c","updated":"2015-11-24 06:44:51.000000000","message":"action \u003d\u003d \u0027add\u0027 or action \u003d\u003d \u0027remove\u0027 is not a single case here. I think they are acceptable.","commit_id":"af71c5f18c92f810f650f37a23278b9248f77945"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"805319a74e8e87be0dba34684ce45c36e6aedbe3","unresolved":false,"context_lines":[{"line_number":264,"context_line":"        fip_ip \u003d fip[\u0027floating_ip_address\u0027]"},{"line_number":265,"context_line":"        fip_fixed_ip \u003d fip[\u0027fixed_ip_address\u0027]"},{"line_number":266,"context_line":"        fip_route_via \u003d self.get_fip_route_dev_and_ip(fip_fixed_ip)[1]"},{"line_number":267,"context_line":"        self._update_fip_route(fip_ip, \u0027add\u0027, fip_route_via)"},{"line_number":268,"context_line":""},{"line_number":269,"context_line":"    def remove_floating_ip(self, ip_cidr):"},{"line_number":270,"context_line":"        fip_ip \u003d str(netaddr.IPNetwork(ip_cidr).ip)"}],"source_content_type":"text/x-python","patch_set":10,"id":"ba8a016a_d6ce7b82","line":267,"updated":"2015-11-24 05:12:26.000000000","message":"Don\u0027t you need the qr device name here for the route?","commit_id":"af71c5f18c92f810f650f37a23278b9248f77945"},{"author":{"_account_id":13667,"name":"ZongKai LI","email":"onionpiece@163.com","username":"lzklibj"},"change_message_id":"bb0184b0e9718b8d69aa42cd89437f52e07ce493","unresolved":false,"context_lines":[{"line_number":264,"context_line":"        fip_ip \u003d fip[\u0027floating_ip_address\u0027]"},{"line_number":265,"context_line":"        fip_fixed_ip \u003d fip[\u0027fixed_ip_address\u0027]"},{"line_number":266,"context_line":"        fip_route_via \u003d self.get_fip_route_dev_and_ip(fip_fixed_ip)[1]"},{"line_number":267,"context_line":"        self._update_fip_route(fip_ip, \u0027add\u0027, fip_route_via)"},{"line_number":268,"context_line":""},{"line_number":269,"context_line":"    def remove_floating_ip(self, ip_cidr):"},{"line_number":270,"context_line":"        fip_ip \u003d str(netaddr.IPNetwork(ip_cidr).ip)"}],"source_content_type":"text/x-python","patch_set":10,"id":"ba8a016a_e04c1244","line":267,"in_reply_to":"ba8a016a_11b64d8a","updated":"2015-11-24 07:45:08.000000000","message":"I will unify the way to add floating IP route in next patch.","commit_id":"af71c5f18c92f810f650f37a23278b9248f77945"},{"author":{"_account_id":13667,"name":"ZongKai LI","email":"onionpiece@163.com","username":"lzklibj"},"change_message_id":"8dd546c57fff5b8b2dee95cd195cfe661f4ff085","unresolved":false,"context_lines":[{"line_number":264,"context_line":"        fip_ip \u003d fip[\u0027floating_ip_address\u0027]"},{"line_number":265,"context_line":"        fip_fixed_ip \u003d fip[\u0027fixed_ip_address\u0027]"},{"line_number":266,"context_line":"        fip_route_via \u003d self.get_fip_route_dev_and_ip(fip_fixed_ip)[1]"},{"line_number":267,"context_line":"        self._update_fip_route(fip_ip, \u0027add\u0027, fip_route_via)"},{"line_number":268,"context_line":""},{"line_number":269,"context_line":"    def remove_floating_ip(self, ip_cidr):"},{"line_number":270,"context_line":"        fip_ip \u003d str(netaddr.IPNetwork(ip_cidr).ip)"}],"source_content_type":"text/x-python","patch_set":10,"id":"ba8a016a_11b64d8a","line":267,"in_reply_to":"ba8a016a_d6ce7b82","updated":"2015-11-24 06:44:51.000000000","message":"nexthop/via is necessary here, while device name is optional.","commit_id":"af71c5f18c92f810f650f37a23278b9248f77945"},{"author":{"_account_id":13667,"name":"ZongKai LI","email":"onionpiece@163.com","username":"lzklibj"},"change_message_id":"a42c5e7525e36d9d947629d3b896cdf96be51759","unresolved":false,"context_lines":[{"line_number":264,"context_line":"        fip_ip \u003d fip[\u0027floating_ip_address\u0027]"},{"line_number":265,"context_line":"        fip_fixed_ip \u003d fip[\u0027fixed_ip_address\u0027]"},{"line_number":266,"context_line":"        fip_route_via \u003d self.get_fip_route_dev_and_ip(fip_fixed_ip)[1]"},{"line_number":267,"context_line":"        self._update_fip_route(fip_ip, \u0027add\u0027, fip_route_via)"},{"line_number":268,"context_line":""},{"line_number":269,"context_line":"    def remove_floating_ip(self, ip_cidr):"},{"line_number":270,"context_line":"        fip_ip \u003d str(netaddr.IPNetwork(ip_cidr).ip)"}],"source_content_type":"text/x-python","patch_set":10,"id":"ba8a016a_e6550a50","line":267,"in_reply_to":"ba8a016a_e04c1244","updated":"2015-11-24 08:49:41.000000000","message":"Done","commit_id":"af71c5f18c92f810f650f37a23278b9248f77945"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"0cac02f877eeaf33e9c8435962d9a84b3bff1ee9","unresolved":false,"context_lines":[{"line_number":248,"context_line":"        self._add_default_gw_virtual_route(ex_gw_port, interface_name)"},{"line_number":249,"context_line":"        self._add_extra_subnet_onlink_routes(ex_gw_port, interface_name)"},{"line_number":250,"context_line":""},{"line_number":251,"context_line":"    def _update_fip_route(self, fip, action, via\u003dNone):"},{"line_number":252,"context_line":"        instance \u003d self._get_keepalived_instance()"},{"line_number":253,"context_line":"        if action \u003d\u003d \u0027add\u0027:"},{"line_number":254,"context_line":"            fip_route \u003d keepalived.KeepalivedVirtualRoute(fip, via)"}],"source_content_type":"text/x-python","patch_set":11,"id":"9a8ffd7b_9f529f12","line":251,"updated":"2015-11-24 21:28:07.000000000","message":"I don\u0027t think it is helping to handle \u0027add\u0027 and remove in this common method.  Honestly, the only thing common in this method is:\n\n  instance \u003d self._get_keepalived_instance()\n\nWhich is a very trivial statement.  Not worth the extra indirection of a private method.  Actually, the first indication to me of this was the presence of the optional via method in such a simple method.\n\nJust inline this method and the whole thing will reduce in complexity.  In fact, the diff will also appear to be smaller because things will line up better.","commit_id":"3187a1d1de659fe51715ff5105fe69398d9d845d"},{"author":{"_account_id":13667,"name":"ZongKai LI","email":"onionpiece@163.com","username":"lzklibj"},"change_message_id":"f1c75dbbde5e05b508ba2df173790d1c1ed99f62","unresolved":false,"context_lines":[{"line_number":248,"context_line":"        self._add_default_gw_virtual_route(ex_gw_port, interface_name)"},{"line_number":249,"context_line":"        self._add_extra_subnet_onlink_routes(ex_gw_port, interface_name)"},{"line_number":250,"context_line":""},{"line_number":251,"context_line":"    def _update_fip_route(self, fip, action, via\u003dNone):"},{"line_number":252,"context_line":"        instance \u003d self._get_keepalived_instance()"},{"line_number":253,"context_line":"        if action \u003d\u003d \u0027add\u0027:"},{"line_number":254,"context_line":"            fip_route \u003d keepalived.KeepalivedVirtualRoute(fip, via)"}],"source_content_type":"text/x-python","patch_set":11,"id":"7a740942_345c1b18","line":251,"in_reply_to":"9a8ffd7b_8ab81a2c","updated":"2015-12-09 10:47:18.000000000","message":"Done","commit_id":"3187a1d1de659fe51715ff5105fe69398d9d845d"},{"author":{"_account_id":13667,"name":"ZongKai LI","email":"onionpiece@163.com","username":"lzklibj"},"change_message_id":"8abdb61cbd303a579de29b1e0962040e6f1e9abb","unresolved":false,"context_lines":[{"line_number":248,"context_line":"        self._add_default_gw_virtual_route(ex_gw_port, interface_name)"},{"line_number":249,"context_line":"        self._add_extra_subnet_onlink_routes(ex_gw_port, interface_name)"},{"line_number":250,"context_line":""},{"line_number":251,"context_line":"    def _update_fip_route(self, fip, action, via\u003dNone):"},{"line_number":252,"context_line":"        instance \u003d self._get_keepalived_instance()"},{"line_number":253,"context_line":"        if action \u003d\u003d \u0027add\u0027:"},{"line_number":254,"context_line":"            fip_route \u003d keepalived.KeepalivedVirtualRoute(fip, via)"}],"source_content_type":"text/x-python","patch_set":11,"id":"9a8ffd7b_8ab81a2c","line":251,"in_reply_to":"9a8ffd7b_9f529f12","updated":"2015-12-01 05:51:01.000000000","message":"OK, I will separate this to two methods for adding and removing cases.","commit_id":"3187a1d1de659fe51715ff5105fe69398d9d845d"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"0cac02f877eeaf33e9c8435962d9a84b3bff1ee9","unresolved":false,"context_lines":[{"line_number":256,"context_line":"        else:"},{"line_number":257,"context_line":"            instance.virtual_routes.fip_routes.pop(fip)"},{"line_number":258,"context_line":""},{"line_number":259,"context_line":"    def get_existing_fips(self):"},{"line_number":260,"context_line":"        instance \u003d self._get_keepalived_instance()"},{"line_number":261,"context_line":"        return set(instance.virtual_routes.fip_routes.keys())"},{"line_number":262,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"9a8ffd7b_1f11cf52","line":259,"updated":"2015-11-24 21:28:07.000000000","message":"This is the new version of get_router_cidrs but it has moved.  Moving it makes it a little more difficult for reviewers to follow.","commit_id":"3187a1d1de659fe51715ff5105fe69398d9d845d"},{"author":{"_account_id":13667,"name":"ZongKai LI","email":"onionpiece@163.com","username":"lzklibj"},"change_message_id":"f1c75dbbde5e05b508ba2df173790d1c1ed99f62","unresolved":false,"context_lines":[{"line_number":256,"context_line":"        else:"},{"line_number":257,"context_line":"            instance.virtual_routes.fip_routes.pop(fip)"},{"line_number":258,"context_line":""},{"line_number":259,"context_line":"    def get_existing_fips(self):"},{"line_number":260,"context_line":"        instance \u003d self._get_keepalived_instance()"},{"line_number":261,"context_line":"        return set(instance.virtual_routes.fip_routes.keys())"},{"line_number":262,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"7a740942_746623ee","line":259,"in_reply_to":"9a8ffd7b_0ac98aac","updated":"2015-12-09 10:47:18.000000000","message":"I move it to L181 now, hope that can help people to review.","commit_id":"3187a1d1de659fe51715ff5105fe69398d9d845d"},{"author":{"_account_id":13667,"name":"ZongKai LI","email":"onionpiece@163.com","username":"lzklibj"},"change_message_id":"8abdb61cbd303a579de29b1e0962040e6f1e9abb","unresolved":false,"context_lines":[{"line_number":256,"context_line":"        else:"},{"line_number":257,"context_line":"            instance.virtual_routes.fip_routes.pop(fip)"},{"line_number":258,"context_line":""},{"line_number":259,"context_line":"    def get_existing_fips(self):"},{"line_number":260,"context_line":"        instance \u003d self._get_keepalived_instance()"},{"line_number":261,"context_line":"        return set(instance.virtual_routes.fip_routes.keys())"},{"line_number":262,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"9a8ffd7b_0ac98aac","line":259,"in_reply_to":"9a8ffd7b_1f11cf52","updated":"2015-12-01 05:51:01.000000000","message":"So, I\u0027d better move it to L181, right ?","commit_id":"3187a1d1de659fe51715ff5105fe69398d9d845d"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"844b21c6512a20c9ca31dea068ef9c27831a0b79","unresolved":false,"context_lines":[{"line_number":408,"context_line":"            self.get_ex_gw_port())"},{"line_number":409,"context_line":"        action \u003d \u0027add\u0027 if state \u003d\u003d \u0027master\u0027 else \u0027remove\u0027"},{"line_number":410,"context_line":"        ip_lib.update_arp_proxies(self.ns_name, existing_fips,"},{"line_number":411,"context_line":"                                  interface_name, action)"}],"source_content_type":"text/x-python","patch_set":17,"id":"1a7b0d38_0920ce9a","line":411,"range":{"start_line":411,"start_character":50,"end_line":411,"end_character":56},"updated":"2015-12-23 02:20:36.000000000","message":"Don\u0027t call update_arp_proxies here.  It should be made private in ip_lib.  Do this instead:\n\n  if state \u003d\u003d \u0027master\u0027:\n      ip_lib.add_arp_proxies(self.ns_name, existing_fips, interface_name)\n  else:\n      ip_lib.remove_arp_proxies(self.ns_name, existing_fips, interface_name)","commit_id":"0ea97018d1c136c07c3b37e7c15812b52d7b3839"}],"neutron/agent/l3/legacy_router.py":[{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"0cac02f877eeaf33e9c8435962d9a84b3bff1ee9","unresolved":false,"context_lines":[{"line_number":22,"context_line":"        fip_ip \u003d common_utils.ip_to_cidr(fip[\u0027floating_ip_address\u0027])"},{"line_number":23,"context_line":"        nexthop \u003d self.get_fip_route_nexthop(fip[\u0027fixed_ip_address\u0027])"},{"line_number":24,"context_line":"        ns_route \u003d ip_lib.IPRoute(self.ns_name)"},{"line_number":25,"context_line":"        ns_route.route.add_route(fip_ip, nexthop)"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"    def remove_floating_ip(self, ip_cidr):"},{"line_number":28,"context_line":"        ns_route \u003d ip_lib.IPRoute(self.ns_name)"}],"source_content_type":"text/x-python","patch_set":11,"id":"9a8ffd7b_9fbb7fc4","line":25,"updated":"2015-11-24 21:28:07.000000000","message":"I think for this to work, it depends on proxy_arp in /proc being turned on for this interface.  Is that true?  (I confirmed by looking at router_info.py)\n\nI\u0027m not sure we can/should count on proxy arp [1].  In fact, I\u0027m certain that we shouldn\u0027t.  The router should definitely not respond to IPs it thinks are on other interfaces.  This would, for example, cause the router to respond to arp requests for private tenant subnets behind the router.  This could open up a security hole.  We cannot do this.\n\nI think we\u0027ll need to do more explicit arp on the interface.  I thought that DVR used \"ip neighbor proxy\" for this but I see that was deprecated and maybe gone entirely.  Hmmm...\n\nI think what we need is to create an arp entry with the publish flag to instruct the stack to respond to arp queries.  Something like this maybe?\n\n$ arp -Ds fip_ip interface_name pub\n\n [1] http://linux-ip.net/html/ether-arp-proxy.html","commit_id":"3187a1d1de659fe51715ff5105fe69398d9d845d"},{"author":{"_account_id":13667,"name":"ZongKai LI","email":"onionpiece@163.com","username":"lzklibj"},"change_message_id":"f1c75dbbde5e05b508ba2df173790d1c1ed99f62","unresolved":false,"context_lines":[{"line_number":22,"context_line":"        fip_ip \u003d common_utils.ip_to_cidr(fip[\u0027floating_ip_address\u0027])"},{"line_number":23,"context_line":"        nexthop \u003d self.get_fip_route_nexthop(fip[\u0027fixed_ip_address\u0027])"},{"line_number":24,"context_line":"        ns_route \u003d ip_lib.IPRoute(self.ns_name)"},{"line_number":25,"context_line":"        ns_route.route.add_route(fip_ip, nexthop)"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"    def remove_floating_ip(self, ip_cidr):"},{"line_number":28,"context_line":"        ns_route \u003d ip_lib.IPRoute(self.ns_name)"}],"source_content_type":"text/x-python","patch_set":11,"id":"7a740942_94a38fff","line":25,"in_reply_to":"9a8ffd7b_4a36b2b5","updated":"2015-12-09 10:47:18.000000000","message":"Done","commit_id":"3187a1d1de659fe51715ff5105fe69398d9d845d"},{"author":{"_account_id":13667,"name":"ZongKai LI","email":"onionpiece@163.com","username":"lzklibj"},"change_message_id":"8abdb61cbd303a579de29b1e0962040e6f1e9abb","unresolved":false,"context_lines":[{"line_number":22,"context_line":"        fip_ip \u003d common_utils.ip_to_cidr(fip[\u0027floating_ip_address\u0027])"},{"line_number":23,"context_line":"        nexthop \u003d self.get_fip_route_nexthop(fip[\u0027fixed_ip_address\u0027])"},{"line_number":24,"context_line":"        ns_route \u003d ip_lib.IPRoute(self.ns_name)"},{"line_number":25,"context_line":"        ns_route.route.add_route(fip_ip, nexthop)"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"    def remove_floating_ip(self, ip_cidr):"},{"line_number":28,"context_line":"        ns_route \u003d ip_lib.IPRoute(self.ns_name)"}],"source_content_type":"text/x-python","patch_set":11,"id":"9a8ffd7b_4a36b2b5","line":25,"in_reply_to":"9a8ffd7b_9fbb7fc4","updated":"2015-12-01 05:51:01.000000000","message":"Thanks for your help, Carl. I manually tested it, it works. I will try to follow comments here to use arp command.","commit_id":"3187a1d1de659fe51715ff5105fe69398d9d845d"},{"author":{"_account_id":6876,"name":"stephen-ma","email":"stephenbma43@gmail.com","username":"stephen-ma"},"change_message_id":"06b940e24c8aab1a8482785850116bc592a0d9a9","unresolved":false,"context_lines":[{"line_number":24,"context_line":"        ns_route \u003d ip_lib.IPRoute(self.ns_name)"},{"line_number":25,"context_line":"        ns_route.route.add_route(fip_ip, nexthop)"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"    def remove_floating_ip(self, ip_cidr):"},{"line_number":28,"context_line":"        ns_route \u003d ip_lib.IPRoute(self.ns_name)"},{"line_number":29,"context_line":"        ns_route.route.delete_route(ip_cidr)"},{"line_number":30,"context_line":"        super(LegacyRouter, self).remove_floating_ip(ip_cidr)"}],"source_content_type":"text/x-python","patch_set":11,"id":"9a8ffd7b_cf3cfe31","line":27,"range":{"start_line":27,"start_character":33,"end_line":27,"end_character":40},"updated":"2015-11-30 23:52:44.000000000","message":"nit: change ip to fip_cidr.","commit_id":"3187a1d1de659fe51715ff5105fe69398d9d845d"},{"author":{"_account_id":13667,"name":"ZongKai LI","email":"onionpiece@163.com","username":"lzklibj"},"change_message_id":"8abdb61cbd303a579de29b1e0962040e6f1e9abb","unresolved":false,"context_lines":[{"line_number":24,"context_line":"        ns_route \u003d ip_lib.IPRoute(self.ns_name)"},{"line_number":25,"context_line":"        ns_route.route.add_route(fip_ip, nexthop)"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"    def remove_floating_ip(self, ip_cidr):"},{"line_number":28,"context_line":"        ns_route \u003d ip_lib.IPRoute(self.ns_name)"},{"line_number":29,"context_line":"        ns_route.route.delete_route(ip_cidr)"},{"line_number":30,"context_line":"        super(LegacyRouter, self).remove_floating_ip(ip_cidr)"}],"source_content_type":"text/x-python","patch_set":11,"id":"9a8ffd7b_2a456e16","line":27,"in_reply_to":"9a8ffd7b_cf3cfe31","updated":"2015-12-01 05:51:01.000000000","message":"ok.","commit_id":"3187a1d1de659fe51715ff5105fe69398d9d845d"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"4f4745b50c414438d1186929d4ba7ed631dbd6c4","unresolved":false,"context_lines":[{"line_number":22,"context_line":"        fip_ip \u003d common_utils.ip_to_cidr(fip[\u0027floating_ip_address\u0027])"},{"line_number":23,"context_line":"        nexthop \u003d self.get_fip_route_nexthop(fip[\u0027fixed_ip_address\u0027])"},{"line_number":24,"context_line":"        ns_route \u003d ip_lib.IPRoute(self.ns_name)"},{"line_number":25,"context_line":"        ns_route.route.add_route(fip_ip, nexthop)"},{"line_number":26,"context_line":"        if not remap:"},{"line_number":27,"context_line":"            ip_lib.add_arp_proxies(self.ns_name, [fip[\u0027floating_ip_address\u0027]],"},{"line_number":28,"context_line":"                                   interface_name)"}],"source_content_type":"text/x-python","patch_set":23,"id":"9a68dd71_e93e9528","line":25,"range":{"start_line":25,"start_character":23,"end_line":25,"end_character":32},"updated":"2016-01-26 17:08:05.000000000","message":"Why are we setting a route here?  Traffic destined to fip_ip gets DNAT applied in the PREROUTING iptables chain to chain the destination to the fixed_ip, right?  By the time packets hit the routing table, they are routed based on the fixed ip.  Why is a next hop necessary?  Is there something I\u0027m not understanding?\n\nEven if DNAT weren\u0027t applied to connection, this route doesn\u0027t make sense to me.  If I understand \"get_fip_router_nexthop\" it returns the gateway address on the subnet","commit_id":"a716d93c9d387141700c0790b1b0bb3f7843dc62"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"12b1daddda2d38560b57a9753f6347f6f9cb3ce0","unresolved":false,"context_lines":[{"line_number":22,"context_line":"        fip_ip \u003d common_utils.ip_to_cidr(fip[\u0027floating_ip_address\u0027])"},{"line_number":23,"context_line":"        nexthop \u003d self.get_fip_route_nexthop(fip[\u0027fixed_ip_address\u0027])"},{"line_number":24,"context_line":"        ns_route \u003d ip_lib.IPRoute(self.ns_name)"},{"line_number":25,"context_line":"        ns_route.route.add_route(fip_ip, nexthop)"},{"line_number":26,"context_line":"        if not remap:"},{"line_number":27,"context_line":"            ip_lib.add_arp_proxies(self.ns_name, [fip[\u0027floating_ip_address\u0027]],"},{"line_number":28,"context_line":"                                   interface_name)"}],"source_content_type":"text/x-python","patch_set":23,"id":"bae84128_7ef4065a","line":25,"range":{"start_line":25,"start_character":23,"end_line":25,"end_character":32},"in_reply_to":"7a5de9d1_ca8171c4","updated":"2016-02-24 02:24:56.000000000","message":"It was my understanding that the \"add_arp_proxies\" call below is supposed to add proxy arp for the floating ip address in a way that doesn\u0027t rely on adding routes to other interfaces.  Does that not work at all?  I thought you had reported that it worked.  I might have to set up an environment myself where I can play with this to understand it.\n\nIs proxy_arp enabled for this interface?  It seems like we\u0027re still relying on linux proxy_arp for this.  Why else would we need this route added here to get arp to work?  We\u0027ve already determined that this route doesn\u0027t actually function as a route.  Linux proxy_arp works with routes:  \"The device performing proxy ARP ... responds for all ARP queries on behalf of IPs reachable on interfaces other than the interface on which the query arrives\"[2]\n\nI thought we were going for a solution that didn\u0027t use the linux proxy arp  mechanism because proxy_arp on Neutron routers would be bad [1].  In fact, proxy_arp should be disabled, IMO because if proxy arp is on, a tenant could add a subnet with public addresses behind the router and potentially steal public IP address traffic with proxy arp on the router.\n\nFrom the manpage for arp(8), it is my understanding that we can use the arp command to make an interface respond to arp queries for a particular address [3]:\n\n   /usr/sbin/arp -i eth0 -Ds 10.0.0.2 eth1 pub\n\n       This will answer ARP requests for 10.0.0.2 on eth0 with the MAC address\n       for eth1.\n\n [1] https://review.openstack.org/#/c/246855/11/neutron/agent/l3/legacy_router.py\n [2] http://linux-ip.net/html/ether-arp-proxy.html\n [3] http://manpages.ubuntu.com/manpages/trusty/man8/arp.8.html","commit_id":"a716d93c9d387141700c0790b1b0bb3f7843dc62"},{"author":{"_account_id":13667,"name":"ZongKai LI","email":"onionpiece@163.com","username":"lzklibj"},"change_message_id":"eca0124319feccd06ab0fc6872d9c6d50f7ae28d","unresolved":false,"context_lines":[{"line_number":22,"context_line":"        fip_ip \u003d common_utils.ip_to_cidr(fip[\u0027floating_ip_address\u0027])"},{"line_number":23,"context_line":"        nexthop \u003d self.get_fip_route_nexthop(fip[\u0027fixed_ip_address\u0027])"},{"line_number":24,"context_line":"        ns_route \u003d ip_lib.IPRoute(self.ns_name)"},{"line_number":25,"context_line":"        ns_route.route.add_route(fip_ip, nexthop)"},{"line_number":26,"context_line":"        if not remap:"},{"line_number":27,"context_line":"            ip_lib.add_arp_proxies(self.ns_name, [fip[\u0027floating_ip_address\u0027]],"},{"line_number":28,"context_line":"                                   interface_name)"}],"source_content_type":"text/x-python","patch_set":23,"id":"7a5de9d1_ca8171c4","line":25,"range":{"start_line":25,"start_character":23,"end_line":25,"end_character":32},"in_reply_to":"9a68dd71_e93e9528","updated":"2016-02-02 05:56:54.000000000","message":"Sorry for reply too late. I have to say I\u0027ve lost some memory of this patch. I tested on my environment with removing the route here, and later I noticed that, the route here is not working for NAT, but for ARP.\n\nThe default route and external network route in router namespace will ignore ARP requests for floatingip. We need an additional route for floatingip to let router namespace \"accept\" ARP requests, and later arp proxy can reply those ARP requests.\n\nI will add a comment here to do simple explain.","commit_id":"a716d93c9d387141700c0790b1b0bb3f7843dc62"},{"author":{"_account_id":13667,"name":"ZongKai LI","email":"onionpiece@163.com","username":"lzklibj"},"change_message_id":"aa194be517f5a9a3e0576d0fb7f73921e8288b32","unresolved":false,"context_lines":[{"line_number":22,"context_line":"        fip_ip \u003d common_utils.ip_to_cidr(fip[\u0027floating_ip_address\u0027])"},{"line_number":23,"context_line":"        nexthop \u003d self.get_fip_route_nexthop(fip[\u0027fixed_ip_address\u0027])"},{"line_number":24,"context_line":"        ns_route \u003d ip_lib.IPRoute(self.ns_name)"},{"line_number":25,"context_line":"        ns_route.route.add_route(fip_ip, nexthop)"},{"line_number":26,"context_line":"        if not remap:"},{"line_number":27,"context_line":"            ip_lib.add_arp_proxies(self.ns_name, [fip[\u0027floating_ip_address\u0027]],"},{"line_number":28,"context_line":"                                   interface_name)"}],"source_content_type":"text/x-python","patch_set":23,"id":"bae84128_c46619ec","line":25,"range":{"start_line":25,"start_character":23,"end_line":25,"end_character":32},"in_reply_to":"bae84128_7ef4065a","updated":"2016-02-24 08:27:05.000000000","message":"Hi, Carl. I\u0027ve checked and tested again on my env, proxy_arp is not been used.\n\n\"The key part of the correct functioning of proxy ARP in a network is that the host breaking a network into two parts has correct routes for all destinations in both halves of the network. If the host which has interfaces in both networks does not have an accurate routing table, IP packets will get dropped on the routing device.\"[1], hope this can have something helpful on understanding.\n\nIn a legacy router case, if we have external network with cidr 10.0.0.0/24, then in qrouter namespace, we will have a route for qg device like \"10.0.0.0/24 dev qg-* ...\". And without adding additional route, floatingip IP address will match that route, that will make packets for floatingip address received from device qg been sent from qg. But logically, we\u0027re hoping those packets can pass through qrouter, the packets path should be: external-\u003eqg-\u003eqr-\u003einside, or inside-\u003eqr-\u003eqg-\u003eexternal.\n\nSo adding additional route for floatingip is try to add a workable route for external network to \"access\" internal network.\n\n[1] http://linux-ip.net/html/adv-proxy-arp.html","commit_id":"a716d93c9d387141700c0790b1b0bb3f7843dc62"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"51145e4ac071af0889001ed946e644ed8d2f516c","unresolved":false,"context_lines":[{"line_number":22,"context_line":"        fip_ip \u003d common_utils.ip_to_cidr(fip[\u0027floating_ip_address\u0027])"},{"line_number":23,"context_line":"        nexthop \u003d self.get_fip_route_nexthop(fip[\u0027fixed_ip_address\u0027])"},{"line_number":24,"context_line":"        ns_route \u003d ip_lib.IPRoute(self.ns_name)"},{"line_number":25,"context_line":"        # Add a route for floatingip to make ARP requests for floatingip"},{"line_number":26,"context_line":"        # can be accepted by router."},{"line_number":27,"context_line":"        ns_route.route.add_route(fip_ip, nexthop)"},{"line_number":28,"context_line":"        if not remap:"},{"line_number":29,"context_line":"            ip_lib.add_arp_proxies(self.ns_name, [fip[\u0027floating_ip_address\u0027]],"}],"source_content_type":"text/x-python","patch_set":27,"id":"bae84128_5e81428e","line":26,"range":{"start_line":25,"start_character":10,"end_line":26,"end_character":35},"updated":"2016-02-24 02:26:58.000000000","message":"See my reply on PS23.  In order to make process on this, we might need to get this running in environment where I can try it out.  @StephenMa, do you think you could set this up somewhere where you and I could poke around with it?","commit_id":"c1f0e471a71922c9911511a7dd5887fc4fb528d7"}],"neutron/agent/l3/router_info.py":[{"author":{"_account_id":13667,"name":"ZongKai LI","email":"onionpiece@163.com","username":"lzklibj"},"change_message_id":"146836b8686e3f2872b6237b073770eea8108a7b","unresolved":false,"context_lines":[{"line_number":213,"context_line":"    def remove_external_gateway_ip(self, device, ip_cidr):"},{"line_number":214,"context_line":"        device.delete_addr_and_conntrack_state(ip_cidr)"},{"line_number":215,"context_line":""},{"line_number":216,"context_line":"    def get_existing_fips(self, device\u003dNone):"},{"line_number":217,"context_line":"        float_rules \u003d self.iptables_manager.get_chain(\u0027nat\u0027, \u0027float-snat\u0027)"},{"line_number":218,"context_line":"        return set([r.rule.split()[-1] for r in float_rules])"},{"line_number":219,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"ba8a016a_e9d07d21","line":216,"updated":"2015-11-18 11:10:19.000000000","message":"for stephen:\nSince router_info has iptables_manager as its attribute, and iptables_manager will store iptable rules added in router namespace, I think it can save some time for us to get rules in float-snat chain by using get_chain, not get_rules_for_table.\n\nget_rules_for_table will try to get rules linux.utils execute command, but not get from rules stored by iptables_manager. But we still need to use get_rules_for_table in dvr_fip_ns scan_fip_* , for when it\u0027s scanning, there is no rule stored by iptables_manage yet.","commit_id":"df2f1ff35a783e70f0a6c458cf0cdbb5da0d5f6f"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"a1ce48ea0cdc7e26cda5205dbc217701edc377bb","unresolved":false,"context_lines":[{"line_number":178,"context_line":"            raise n_exc.FloatingIpSetupException("},{"line_number":179,"context_line":"                \u0027L3 agent failure to setup NAT for floating IPs\u0027)"},{"line_number":180,"context_line":""},{"line_number":181,"context_line":"    def add_floating_ip(self, fip, interface_name, device):"},{"line_number":182,"context_line":"        # As GARP is processed in a distinct thread the call below"},{"line_number":183,"context_line":"        # won\u0027t raise an exception to be handled."},{"line_number":184,"context_line":"        fip_ip \u003d str(netaddr.IPNetwork(fip[\u0027floating_ip_address\u0027]))"}],"source_content_type":"text/x-python","patch_set":4,"id":"ba8a016a_9d721910","line":181,"updated":"2015-11-19 07:21:31.000000000","message":"Is the device parameter still needed here? Since this is the base change, if you leave the parameter here, we may need another refactor for the code after all changes are done.\nSo, i think it is better to just remove it here.","commit_id":"3eeaba395addfead4be4bdf04e646a674b6743cf"},{"author":{"_account_id":13667,"name":"ZongKai LI","email":"onionpiece@163.com","username":"lzklibj"},"change_message_id":"dc1253d93cf2179d9c3df328f858c8d210a2290e","unresolved":false,"context_lines":[{"line_number":178,"context_line":"            raise n_exc.FloatingIpSetupException("},{"line_number":179,"context_line":"                \u0027L3 agent failure to setup NAT for floating IPs\u0027)"},{"line_number":180,"context_line":""},{"line_number":181,"context_line":"    def add_floating_ip(self, fip, interface_name, device):"},{"line_number":182,"context_line":"        # As GARP is processed in a distinct thread the call below"},{"line_number":183,"context_line":"        # won\u0027t raise an exception to be handled."},{"line_number":184,"context_line":"        fip_ip \u003d str(netaddr.IPNetwork(fip[\u0027floating_ip_address\u0027]))"}],"source_content_type":"text/x-python","patch_set":4,"id":"ba8a016a_9572ed0f","line":181,"in_reply_to":"ba8a016a_9d721910","updated":"2015-11-19 08:41:01.000000000","message":"Done","commit_id":"3eeaba395addfead4be4bdf04e646a674b6743cf"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"a1ce48ea0cdc7e26cda5205dbc217701edc377bb","unresolved":false,"context_lines":[{"line_number":181,"context_line":"    def add_floating_ip(self, fip, interface_name, device):"},{"line_number":182,"context_line":"        # As GARP is processed in a distinct thread the call below"},{"line_number":183,"context_line":"        # won\u0027t raise an exception to be handled."},{"line_number":184,"context_line":"        fip_ip \u003d str(netaddr.IPNetwork(fip[\u0027floating_ip_address\u0027]))"},{"line_number":185,"context_line":"        # FIXME: adding FIP route onto LOOPBACK device should be a"},{"line_number":186,"context_line":"        #        temporary way to do arp proxy, we need find a better way."},{"line_number":187,"context_line":"        device \u003d ip_lib.IPDevice(ip_lib.LOOPBACK_DEVNAME, self.ns_name)"}],"source_content_type":"text/x-python","patch_set":4,"id":"ba8a016a_9da8d978","line":184,"updated":"2015-11-19 07:21:31.000000000","message":"Try using common_utils.ip_to_cidr(ip_address), here.","commit_id":"3eeaba395addfead4be4bdf04e646a674b6743cf"},{"author":{"_account_id":13667,"name":"ZongKai LI","email":"onionpiece@163.com","username":"lzklibj"},"change_message_id":"dc1253d93cf2179d9c3df328f858c8d210a2290e","unresolved":false,"context_lines":[{"line_number":181,"context_line":"    def add_floating_ip(self, fip, interface_name, device):"},{"line_number":182,"context_line":"        # As GARP is processed in a distinct thread the call below"},{"line_number":183,"context_line":"        # won\u0027t raise an exception to be handled."},{"line_number":184,"context_line":"        fip_ip \u003d str(netaddr.IPNetwork(fip[\u0027floating_ip_address\u0027]))"},{"line_number":185,"context_line":"        # FIXME: adding FIP route onto LOOPBACK device should be a"},{"line_number":186,"context_line":"        #        temporary way to do arp proxy, we need find a better way."},{"line_number":187,"context_line":"        device \u003d ip_lib.IPDevice(ip_lib.LOOPBACK_DEVNAME, self.ns_name)"}],"source_content_type":"text/x-python","patch_set":4,"id":"ba8a016a_155d9d78","line":184,"in_reply_to":"ba8a016a_9da8d978","updated":"2015-11-19 08:41:01.000000000","message":"Done","commit_id":"3eeaba395addfead4be4bdf04e646a674b6743cf"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"a1ce48ea0cdc7e26cda5205dbc217701edc377bb","unresolved":false,"context_lines":[{"line_number":183,"context_line":"        # won\u0027t raise an exception to be handled."},{"line_number":184,"context_line":"        fip_ip \u003d str(netaddr.IPNetwork(fip[\u0027floating_ip_address\u0027]))"},{"line_number":185,"context_line":"        # FIXME: adding FIP route onto LOOPBACK device should be a"},{"line_number":186,"context_line":"        #        temporary way to do arp proxy, we need find a better way."},{"line_number":187,"context_line":"        device \u003d ip_lib.IPDevice(ip_lib.LOOPBACK_DEVNAME, self.ns_name)"},{"line_number":188,"context_line":"        device.route.add_route(fip_ip)"},{"line_number":189,"context_line":"        ip_lib.send_ip_addr_adv_notif(self.ns_name,"}],"source_content_type":"text/x-python","patch_set":4,"id":"ba8a016a_5dbd91b5","line":186,"updated":"2015-11-19 07:21:31.000000000","message":"Indention is not needed here.","commit_id":"3eeaba395addfead4be4bdf04e646a674b6743cf"},{"author":{"_account_id":13667,"name":"ZongKai LI","email":"onionpiece@163.com","username":"lzklibj"},"change_message_id":"dc1253d93cf2179d9c3df328f858c8d210a2290e","unresolved":false,"context_lines":[{"line_number":183,"context_line":"        # won\u0027t raise an exception to be handled."},{"line_number":184,"context_line":"        fip_ip \u003d str(netaddr.IPNetwork(fip[\u0027floating_ip_address\u0027]))"},{"line_number":185,"context_line":"        # FIXME: adding FIP route onto LOOPBACK device should be a"},{"line_number":186,"context_line":"        #        temporary way to do arp proxy, we need find a better way."},{"line_number":187,"context_line":"        device \u003d ip_lib.IPDevice(ip_lib.LOOPBACK_DEVNAME, self.ns_name)"},{"line_number":188,"context_line":"        device.route.add_route(fip_ip)"},{"line_number":189,"context_line":"        ip_lib.send_ip_addr_adv_notif(self.ns_name,"}],"source_content_type":"text/x-python","patch_set":4,"id":"ba8a016a_b556115a","line":186,"in_reply_to":"ba8a016a_5dbd91b5","updated":"2015-11-19 08:41:01.000000000","message":"Done","commit_id":"3eeaba395addfead4be4bdf04e646a674b6743cf"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"a1ce48ea0cdc7e26cda5205dbc217701edc377bb","unresolved":false,"context_lines":[{"line_number":192,"context_line":"                                      self.agent_conf)"},{"line_number":193,"context_line":"        return l3_constants.FLOATINGIP_STATUS_ACTIVE"},{"line_number":194,"context_line":""},{"line_number":195,"context_line":"    def remove_floating_ip(self, device, ip_cidr):"},{"line_number":196,"context_line":"        device \u003d ip_lib.IPDevice(ip_lib.LOOPBACK_DEVNAME, self.ns_name)"},{"line_number":197,"context_line":"        device.route.delete_route(ip_cidr)"},{"line_number":198,"context_line":"        device.delete_conntrack_state(ip_cidr)"}],"source_content_type":"text/x-python","patch_set":4,"id":"ba8a016a_5d67d14c","line":195,"updated":"2015-11-19 07:21:31.000000000","message":"Is the device parameter still needed here? If not, maybe you can remove it.","commit_id":"3eeaba395addfead4be4bdf04e646a674b6743cf"},{"author":{"_account_id":13667,"name":"ZongKai LI","email":"onionpiece@163.com","username":"lzklibj"},"change_message_id":"dc1253d93cf2179d9c3df328f858c8d210a2290e","unresolved":false,"context_lines":[{"line_number":192,"context_line":"                                      self.agent_conf)"},{"line_number":193,"context_line":"        return l3_constants.FLOATINGIP_STATUS_ACTIVE"},{"line_number":194,"context_line":""},{"line_number":195,"context_line":"    def remove_floating_ip(self, device, ip_cidr):"},{"line_number":196,"context_line":"        device \u003d ip_lib.IPDevice(ip_lib.LOOPBACK_DEVNAME, self.ns_name)"},{"line_number":197,"context_line":"        device.route.delete_route(ip_cidr)"},{"line_number":198,"context_line":"        device.delete_conntrack_state(ip_cidr)"}],"source_content_type":"text/x-python","patch_set":4,"id":"ba8a016a_754c29c8","line":195,"in_reply_to":"ba8a016a_5d67d14c","updated":"2015-11-19 08:41:01.000000000","message":"Done","commit_id":"3eeaba395addfead4be4bdf04e646a674b6743cf"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"a1ce48ea0cdc7e26cda5205dbc217701edc377bb","unresolved":false,"context_lines":[{"line_number":200,"context_line":"    def remove_external_gateway_ip(self, device, ip_cidr):"},{"line_number":201,"context_line":"        device.delete_addr_and_conntrack_state(ip_cidr)"},{"line_number":202,"context_line":""},{"line_number":203,"context_line":"    def get_existing_fips(self, device\u003dNone):"},{"line_number":204,"context_line":"        float_rules \u003d self.iptables_manager.get_chain(\u0027nat\u0027, \u0027float-snat\u0027)"},{"line_number":205,"context_line":"        return set([r.rule.split()[-1] for r in float_rules])"},{"line_number":206,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"ba8a016a_9df77958","line":203,"updated":"2015-11-19 07:21:31.000000000","message":"The same as my last comment.","commit_id":"3eeaba395addfead4be4bdf04e646a674b6743cf"},{"author":{"_account_id":13667,"name":"ZongKai LI","email":"onionpiece@163.com","username":"lzklibj"},"change_message_id":"dc1253d93cf2179d9c3df328f858c8d210a2290e","unresolved":false,"context_lines":[{"line_number":200,"context_line":"    def remove_external_gateway_ip(self, device, ip_cidr):"},{"line_number":201,"context_line":"        device.delete_addr_and_conntrack_state(ip_cidr)"},{"line_number":202,"context_line":""},{"line_number":203,"context_line":"    def get_existing_fips(self, device\u003dNone):"},{"line_number":204,"context_line":"        float_rules \u003d self.iptables_manager.get_chain(\u0027nat\u0027, \u0027float-snat\u0027)"},{"line_number":205,"context_line":"        return set([r.rule.split()[-1] for r in float_rules])"},{"line_number":206,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"ba8a016a_35c9c126","line":203,"in_reply_to":"ba8a016a_9df77958","updated":"2015-11-19 08:41:01.000000000","message":"Done","commit_id":"3eeaba395addfead4be4bdf04e646a674b6743cf"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"a1ce48ea0cdc7e26cda5205dbc217701edc377bb","unresolved":false,"context_lines":[{"line_number":211,"context_line":""},{"line_number":212,"context_line":"    def process_floating_ip_addresses(self, interface_name,"},{"line_number":213,"context_line":"                                      existing_fips\u003dNone):"},{"line_number":214,"context_line":"        \"\"\"Configure IP addresses on router\u0027s external gateway interface."},{"line_number":215,"context_line":""},{"line_number":216,"context_line":"        Ensures addresses for existing floating IPs and cleans up"},{"line_number":217,"context_line":"        those that should not longer be configured."}],"source_content_type":"text/x-python","patch_set":4,"id":"ba8a016a_685455fe","line":214,"updated":"2015-11-19 07:21:31.000000000","message":"The comment here may need  update.","commit_id":"3eeaba395addfead4be4bdf04e646a674b6743cf"},{"author":{"_account_id":13667,"name":"ZongKai LI","email":"onionpiece@163.com","username":"lzklibj"},"change_message_id":"dc1253d93cf2179d9c3df328f858c8d210a2290e","unresolved":false,"context_lines":[{"line_number":211,"context_line":""},{"line_number":212,"context_line":"    def process_floating_ip_addresses(self, interface_name,"},{"line_number":213,"context_line":"                                      existing_fips\u003dNone):"},{"line_number":214,"context_line":"        \"\"\"Configure IP addresses on router\u0027s external gateway interface."},{"line_number":215,"context_line":""},{"line_number":216,"context_line":"        Ensures addresses for existing floating IPs and cleans up"},{"line_number":217,"context_line":"        those that should not longer be configured."}],"source_content_type":"text/x-python","patch_set":4,"id":"ba8a016a_d5dab5e7","line":214,"in_reply_to":"ba8a016a_685455fe","updated":"2015-11-19 08:41:01.000000000","message":"Done","commit_id":"3eeaba395addfead4be4bdf04e646a674b6743cf"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"a1ce48ea0cdc7e26cda5205dbc217701edc377bb","unresolved":false,"context_lines":[{"line_number":247,"context_line":"                # that\u0027s how the caller determines that it was removed"},{"line_number":248,"context_line":"                fip_statuses[fip[\u0027id\u0027]] \u003d FLOATINGIP_STATUS_NOCHANGE"},{"line_number":249,"context_line":"        fips_to_remove \u003d existing_fips - new_fips"},{"line_number":250,"context_line":"        if fips_to_remove:"},{"line_number":251,"context_line":"            for ip in fips_to_remove:"},{"line_number":252,"context_line":"                ip_cidr \u003d common_utils.ip_to_cidr(ip)"},{"line_number":253,"context_line":"                LOG.debug(\"Removing floating ip %s\", ip_cidr)"}],"source_content_type":"text/x-python","patch_set":4,"id":"ba8a016a_dd0bc128","line":250,"updated":"2015-11-19 07:21:31.000000000","message":"I think this if check is meaningless, if fips_to_remove is empty, the for loop will not be executed.","commit_id":"3eeaba395addfead4be4bdf04e646a674b6743cf"},{"author":{"_account_id":13667,"name":"ZongKai LI","email":"onionpiece@163.com","username":"lzklibj"},"change_message_id":"dc1253d93cf2179d9c3df328f858c8d210a2290e","unresolved":false,"context_lines":[{"line_number":247,"context_line":"                # that\u0027s how the caller determines that it was removed"},{"line_number":248,"context_line":"                fip_statuses[fip[\u0027id\u0027]] \u003d FLOATINGIP_STATUS_NOCHANGE"},{"line_number":249,"context_line":"        fips_to_remove \u003d existing_fips - new_fips"},{"line_number":250,"context_line":"        if fips_to_remove:"},{"line_number":251,"context_line":"            for ip in fips_to_remove:"},{"line_number":252,"context_line":"                ip_cidr \u003d common_utils.ip_to_cidr(ip)"},{"line_number":253,"context_line":"                LOG.debug(\"Removing floating ip %s\", ip_cidr)"}],"source_content_type":"text/x-python","patch_set":4,"id":"ba8a016a_75e3c9a4","line":250,"in_reply_to":"ba8a016a_dd0bc128","updated":"2015-11-19 08:41:01.000000000","message":"Done","commit_id":"3eeaba395addfead4be4bdf04e646a674b6743cf"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"805319a74e8e87be0dba34684ce45c36e6aedbe3","unresolved":false,"context_lines":[{"line_number":187,"context_line":"        intf_dev \u003d self.get_internal_device_name(intf_id)"},{"line_number":188,"context_line":"        return intf_dev, intf_ip"},{"line_number":189,"context_line":""},{"line_number":190,"context_line":"    def add_floating_ip(self, fip):"},{"line_number":191,"context_line":"        # As GARP is processed in a distinct thread the call below"},{"line_number":192,"context_line":"        # won\u0027t raise an exception to be handled."},{"line_number":193,"context_line":"        fip_ip \u003d common_utils.ip_to_cidr(fip[\u0027floating_ip_address\u0027])"}],"source_content_type":"text/x-python","patch_set":10,"id":"ba8a016a_d668bbbc","line":190,"updated":"2015-11-24 05:12:26.000000000","message":"looks like the content of this method will only be used by legacy router, so why adding it here, and leave an empty class in legacy_router.py\n\nrouter_info should have common content of all types of router.","commit_id":"af71c5f18c92f810f650f37a23278b9248f77945"},{"author":{"_account_id":13667,"name":"ZongKai LI","email":"onionpiece@163.com","username":"lzklibj"},"change_message_id":"a42c5e7525e36d9d947629d3b896cdf96be51759","unresolved":false,"context_lines":[{"line_number":187,"context_line":"        intf_dev \u003d self.get_internal_device_name(intf_id)"},{"line_number":188,"context_line":"        return intf_dev, intf_ip"},{"line_number":189,"context_line":""},{"line_number":190,"context_line":"    def add_floating_ip(self, fip):"},{"line_number":191,"context_line":"        # As GARP is processed in a distinct thread the call below"},{"line_number":192,"context_line":"        # won\u0027t raise an exception to be handled."},{"line_number":193,"context_line":"        fip_ip \u003d common_utils.ip_to_cidr(fip[\u0027floating_ip_address\u0027])"}],"source_content_type":"text/x-python","patch_set":10,"id":"ba8a016a_066cce33","line":190,"in_reply_to":"ba8a016a_912f9da0","updated":"2015-11-24 08:49:41.000000000","message":"Done","commit_id":"af71c5f18c92f810f650f37a23278b9248f77945"},{"author":{"_account_id":13667,"name":"ZongKai LI","email":"onionpiece@163.com","username":"lzklibj"},"change_message_id":"8dd546c57fff5b8b2dee95cd195cfe661f4ff085","unresolved":false,"context_lines":[{"line_number":187,"context_line":"        intf_dev \u003d self.get_internal_device_name(intf_id)"},{"line_number":188,"context_line":"        return intf_dev, intf_ip"},{"line_number":189,"context_line":""},{"line_number":190,"context_line":"    def add_floating_ip(self, fip):"},{"line_number":191,"context_line":"        # As GARP is processed in a distinct thread the call below"},{"line_number":192,"context_line":"        # won\u0027t raise an exception to be handled."},{"line_number":193,"context_line":"        fip_ip \u003d common_utils.ip_to_cidr(fip[\u0027floating_ip_address\u0027])"}],"source_content_type":"text/x-python","patch_set":10,"id":"ba8a016a_912f9da0","line":190,"in_reply_to":"ba8a016a_d668bbbc","updated":"2015-11-24 06:44:51.000000000","message":"Make sense to me. I will do it later.","commit_id":"af71c5f18c92f810f650f37a23278b9248f77945"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"805319a74e8e87be0dba34684ce45c36e6aedbe3","unresolved":false,"context_lines":[{"line_number":189,"context_line":""},{"line_number":190,"context_line":"    def add_floating_ip(self, fip):"},{"line_number":191,"context_line":"        # As GARP is processed in a distinct thread the call below"},{"line_number":192,"context_line":"        # won\u0027t raise an exception to be handled."},{"line_number":193,"context_line":"        fip_ip \u003d common_utils.ip_to_cidr(fip[\u0027floating_ip_address\u0027])"},{"line_number":194,"context_line":"        fip_route_dev, fip_route_via \u003d self.get_fip_route_dev_and_ip("},{"line_number":195,"context_line":"            fip[\u0027fixed_ip_address\u0027])"}],"source_content_type":"text/x-python","patch_set":10,"id":"ba8a016a_d36d2d33","line":192,"updated":"2015-11-24 05:12:26.000000000","message":"Is this comment still needed? Without context, it may cause confuse.","commit_id":"af71c5f18c92f810f650f37a23278b9248f77945"},{"author":{"_account_id":13667,"name":"ZongKai LI","email":"onionpiece@163.com","username":"lzklibj"},"change_message_id":"a42c5e7525e36d9d947629d3b896cdf96be51759","unresolved":false,"context_lines":[{"line_number":189,"context_line":""},{"line_number":190,"context_line":"    def add_floating_ip(self, fip):"},{"line_number":191,"context_line":"        # As GARP is processed in a distinct thread the call below"},{"line_number":192,"context_line":"        # won\u0027t raise an exception to be handled."},{"line_number":193,"context_line":"        fip_ip \u003d common_utils.ip_to_cidr(fip[\u0027floating_ip_address\u0027])"},{"line_number":194,"context_line":"        fip_route_dev, fip_route_via \u003d self.get_fip_route_dev_and_ip("},{"line_number":195,"context_line":"            fip[\u0027fixed_ip_address\u0027])"}],"source_content_type":"text/x-python","patch_set":10,"id":"ba8a016a_a674e2fd","line":192,"in_reply_to":"ba8a016a_113badec","updated":"2015-11-24 08:49:41.000000000","message":"Done","commit_id":"af71c5f18c92f810f650f37a23278b9248f77945"},{"author":{"_account_id":13667,"name":"ZongKai LI","email":"onionpiece@163.com","username":"lzklibj"},"change_message_id":"8dd546c57fff5b8b2dee95cd195cfe661f4ff085","unresolved":false,"context_lines":[{"line_number":189,"context_line":""},{"line_number":190,"context_line":"    def add_floating_ip(self, fip):"},{"line_number":191,"context_line":"        # As GARP is processed in a distinct thread the call below"},{"line_number":192,"context_line":"        # won\u0027t raise an exception to be handled."},{"line_number":193,"context_line":"        fip_ip \u003d common_utils.ip_to_cidr(fip[\u0027floating_ip_address\u0027])"},{"line_number":194,"context_line":"        fip_route_dev, fip_route_via \u003d self.get_fip_route_dev_and_ip("},{"line_number":195,"context_line":"            fip[\u0027fixed_ip_address\u0027])"}],"source_content_type":"text/x-python","patch_set":10,"id":"ba8a016a_113badec","line":192,"in_reply_to":"ba8a016a_d36d2d33","updated":"2015-11-24 06:44:51.000000000","message":"I will remove it.","commit_id":"af71c5f18c92f810f650f37a23278b9248f77945"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"805319a74e8e87be0dba34684ce45c36e6aedbe3","unresolved":false,"context_lines":[{"line_number":215,"context_line":"        ip_wrapper.netns.execute(cmd, check_exit_code\u003dFalse)"},{"line_number":216,"context_line":""},{"line_number":217,"context_line":"    def process_floating_ip_addresses(self, interface_name,"},{"line_number":218,"context_line":"                                      existing_fips\u003dNone):"},{"line_number":219,"context_line":"        \"\"\"Configure new added and cleanup removed floating IP addresses\"\"\""},{"line_number":220,"context_line":""},{"line_number":221,"context_line":"        fip_statuses \u003d {}"}],"source_content_type":"text/x-python","patch_set":10,"id":"ba8a016a_36f6e7f9","line":218,"updated":"2015-11-24 05:12:26.000000000","message":"Why add parameter \"existing_fips\" here? The only place to use it is line 676, and it is assigned with \"self.get_existing_fips()\" in line 667.\n\nSo, it is ok to just use the parameter list like the old method. And get the existing_fips in this method, as line 228.","commit_id":"af71c5f18c92f810f650f37a23278b9248f77945"},{"author":{"_account_id":13667,"name":"ZongKai LI","email":"onionpiece@163.com","username":"lzklibj"},"change_message_id":"8dd546c57fff5b8b2dee95cd195cfe661f4ff085","unresolved":false,"context_lines":[{"line_number":215,"context_line":"        ip_wrapper.netns.execute(cmd, check_exit_code\u003dFalse)"},{"line_number":216,"context_line":""},{"line_number":217,"context_line":"    def process_floating_ip_addresses(self, interface_name,"},{"line_number":218,"context_line":"                                      existing_fips\u003dNone):"},{"line_number":219,"context_line":"        \"\"\"Configure new added and cleanup removed floating IP addresses\"\"\""},{"line_number":220,"context_line":""},{"line_number":221,"context_line":"        fip_statuses \u003d {}"}],"source_content_type":"text/x-python","patch_set":10,"id":"ba8a016a_0c5876f9","line":218,"in_reply_to":"ba8a016a_36f6e7f9","updated":"2015-11-24 06:44:51.000000000","message":"Indeed I add it for https://github.com/openstack/neutron/blob/master/neutron/agent/l3/dvr_local_router.py#L406 .\n\nNow we\u0027re trying to get existing floating IPs from iptable_manager, so get_existing_fips is added in L667, just before process_snat_dnat_for_fip which will adding/removing iptable rules for floating IPs, and that will affect records stored by iptable_manager.","commit_id":"af71c5f18c92f810f650f37a23278b9248f77945"},{"author":{"_account_id":6876,"name":"stephen-ma","email":"stephenbma43@gmail.com","username":"stephen-ma"},"change_message_id":"06b940e24c8aab1a8482785850116bc592a0d9a9","unresolved":false,"context_lines":[{"line_number":174,"context_line":"            raise n_exc.FloatingIpSetupException("},{"line_number":175,"context_line":"                \u0027L3 agent failure to setup NAT for floating IPs\u0027)"},{"line_number":176,"context_line":""},{"line_number":177,"context_line":"    def get_fip_route_nexthop(self, fip_fixed_ip):"},{"line_number":178,"context_line":"        fixed_ip \u003d netaddr.IPAddress(fip_fixed_ip)"},{"line_number":179,"context_line":"        for intf in self.router[\u0027_interfaces\u0027]:"},{"line_number":180,"context_line":"            for s in intf[\u0027subnets\u0027]:"}],"source_content_type":"text/x-python","patch_set":11,"id":"9a8ffd7b_6f763240","line":177,"range":{"start_line":177,"start_character":8,"end_line":177,"end_character":29},"updated":"2015-11-30 23:52:44.000000000","message":"nit: it is easier to read the other changes if name is get_fip_route_nexthop_ip.","commit_id":"3187a1d1de659fe51715ff5105fe69398d9d845d"},{"author":{"_account_id":13667,"name":"ZongKai LI","email":"onionpiece@163.com","username":"lzklibj"},"change_message_id":"f1c75dbbde5e05b508ba2df173790d1c1ed99f62","unresolved":false,"context_lines":[{"line_number":174,"context_line":"            raise n_exc.FloatingIpSetupException("},{"line_number":175,"context_line":"                \u0027L3 agent failure to setup NAT for floating IPs\u0027)"},{"line_number":176,"context_line":""},{"line_number":177,"context_line":"    def get_fip_route_nexthop(self, fip_fixed_ip):"},{"line_number":178,"context_line":"        fixed_ip \u003d netaddr.IPAddress(fip_fixed_ip)"},{"line_number":179,"context_line":"        for intf in self.router[\u0027_interfaces\u0027]:"},{"line_number":180,"context_line":"            for s in intf[\u0027subnets\u0027]:"}],"source_content_type":"text/x-python","patch_set":11,"id":"7a740942_b4424b0d","line":177,"in_reply_to":"9a8ffd7b_6f763240","updated":"2015-12-09 10:47:18.000000000","message":"I think route nexthop is clear enough. Thanks. : )","commit_id":"3187a1d1de659fe51715ff5105fe69398d9d845d"},{"author":{"_account_id":6876,"name":"stephen-ma","email":"stephenbma43@gmail.com","username":"stephen-ma"},"change_message_id":"06b940e24c8aab1a8482785850116bc592a0d9a9","unresolved":false,"context_lines":[{"line_number":195,"context_line":"        float_rules \u003d self.iptables_manager.get_chain(\u0027nat\u0027, \u0027float-snat\u0027)"},{"line_number":196,"context_line":"        return set([r.rule.split()[-1] for r in float_rules])"},{"line_number":197,"context_line":""},{"line_number":198,"context_line":"    def config_fip_proxy_arp(self, interface_name):"},{"line_number":199,"context_line":"        cmd \u003d [\u0027sysctl\u0027, \u0027-w\u0027, \u0027net.ipv4.conf.%s.proxy_arp\u003d1\u0027 % interface_name]"},{"line_number":200,"context_line":"        ip_wrapper \u003d ip_lib.IPWrapper(namespace\u003dself.ns_name)"},{"line_number":201,"context_line":"        ip_wrapper.netns.execute(cmd, check_exit_code\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":11,"id":"9a8ffd7b_0e10c74d","line":198,"range":{"start_line":198,"start_character":49,"end_line":198,"end_character":51},"updated":"2015-11-30 23:52:44.000000000","message":"I am testing change along with https://review.openstack.org/#/c/246894 on a DVR configuration. I commented out the body of this method and reran the L3-agent. I still have connectivity to the VMs from the outside. All the net.ipv4.conf.*.proxy_arp values are 0.   Is this method really needed for DVR?","commit_id":"3187a1d1de659fe51715ff5105fe69398d9d845d"},{"author":{"_account_id":13667,"name":"ZongKai LI","email":"onionpiece@163.com","username":"lzklibj"},"change_message_id":"8abdb61cbd303a579de29b1e0962040e6f1e9abb","unresolved":false,"context_lines":[{"line_number":195,"context_line":"        float_rules \u003d self.iptables_manager.get_chain(\u0027nat\u0027, \u0027float-snat\u0027)"},{"line_number":196,"context_line":"        return set([r.rule.split()[-1] for r in float_rules])"},{"line_number":197,"context_line":""},{"line_number":198,"context_line":"    def config_fip_proxy_arp(self, interface_name):"},{"line_number":199,"context_line":"        cmd \u003d [\u0027sysctl\u0027, \u0027-w\u0027, \u0027net.ipv4.conf.%s.proxy_arp\u003d1\u0027 % interface_name]"},{"line_number":200,"context_line":"        ip_wrapper \u003d ip_lib.IPWrapper(namespace\u003dself.ns_name)"},{"line_number":201,"context_line":"        ip_wrapper.netns.execute(cmd, check_exit_code\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":11,"id":"9a8ffd7b_0a250a5b","line":198,"in_reply_to":"9a8ffd7b_0e10c74d","updated":"2015-12-01 05:51:01.000000000","message":"I think you\u0027ve get the point, this method is not for DVR, it\u0027s for centralized router. And per Carl\u0027s comments, we will no longer use this method and sysctl proxy_arp.","commit_id":"3187a1d1de659fe51715ff5105fe69398d9d845d"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"0cac02f877eeaf33e9c8435962d9a84b3bff1ee9","unresolved":false,"context_lines":[{"line_number":196,"context_line":"        return set([r.rule.split()[-1] for r in float_rules])"},{"line_number":197,"context_line":""},{"line_number":198,"context_line":"    def config_fip_proxy_arp(self, interface_name):"},{"line_number":199,"context_line":"        cmd \u003d [\u0027sysctl\u0027, \u0027-w\u0027, \u0027net.ipv4.conf.%s.proxy_arp\u003d1\u0027 % interface_name]"},{"line_number":200,"context_line":"        ip_wrapper \u003d ip_lib.IPWrapper(namespace\u003dself.ns_name)"},{"line_number":201,"context_line":"        ip_wrapper.netns.execute(cmd, check_exit_code\u003dFalse)"},{"line_number":202,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"9a8ffd7b_9abf0dd0","line":199,"updated":"2015-11-24 21:28:07.000000000","message":"We cannot do this.  This sets up the router to be essentially a psuedo-bridge.  We cannot have that.  I think we need to look at managing specific arp entries for these.  See my other comments.","commit_id":"3187a1d1de659fe51715ff5105fe69398d9d845d"},{"author":{"_account_id":13667,"name":"ZongKai LI","email":"onionpiece@163.com","username":"lzklibj"},"change_message_id":"f1c75dbbde5e05b508ba2df173790d1c1ed99f62","unresolved":false,"context_lines":[{"line_number":196,"context_line":"        return set([r.rule.split()[-1] for r in float_rules])"},{"line_number":197,"context_line":""},{"line_number":198,"context_line":"    def config_fip_proxy_arp(self, interface_name):"},{"line_number":199,"context_line":"        cmd \u003d [\u0027sysctl\u0027, \u0027-w\u0027, \u0027net.ipv4.conf.%s.proxy_arp\u003d1\u0027 % interface_name]"},{"line_number":200,"context_line":"        ip_wrapper \u003d ip_lib.IPWrapper(namespace\u003dself.ns_name)"},{"line_number":201,"context_line":"        ip_wrapper.netns.execute(cmd, check_exit_code\u003dFalse)"},{"line_number":202,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"7a740942_344f3b35","line":199,"in_reply_to":"9a8ffd7b_8a19fa24","updated":"2015-12-09 10:47:18.000000000","message":"Done","commit_id":"3187a1d1de659fe51715ff5105fe69398d9d845d"},{"author":{"_account_id":13667,"name":"ZongKai LI","email":"onionpiece@163.com","username":"lzklibj"},"change_message_id":"8abdb61cbd303a579de29b1e0962040e6f1e9abb","unresolved":false,"context_lines":[{"line_number":196,"context_line":"        return set([r.rule.split()[-1] for r in float_rules])"},{"line_number":197,"context_line":""},{"line_number":198,"context_line":"    def config_fip_proxy_arp(self, interface_name):"},{"line_number":199,"context_line":"        cmd \u003d [\u0027sysctl\u0027, \u0027-w\u0027, \u0027net.ipv4.conf.%s.proxy_arp\u003d1\u0027 % interface_name]"},{"line_number":200,"context_line":"        ip_wrapper \u003d ip_lib.IPWrapper(namespace\u003dself.ns_name)"},{"line_number":201,"context_line":"        ip_wrapper.netns.execute(cmd, check_exit_code\u003dFalse)"},{"line_number":202,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"9a8ffd7b_8a19fa24","line":199,"in_reply_to":"9a8ffd7b_9abf0dd0","updated":"2015-12-01 05:51:01.000000000","message":"OK, I will use arp command to replace this.","commit_id":"3187a1d1de659fe51715ff5105fe69398d9d845d"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"ba6476fee3d8c378196e3625b74356fbd2e70b97","unresolved":false,"context_lines":[{"line_number":176,"context_line":""},{"line_number":177,"context_line":"    def get_fip_route_nexthop(self, fip_fixed_ip):"},{"line_number":178,"context_line":"        fixed_ip \u003d netaddr.IPAddress(fip_fixed_ip)"},{"line_number":179,"context_line":"        for intf in self.router[\u0027_interfaces\u0027]:"},{"line_number":180,"context_line":"            for s in intf[\u0027subnets\u0027]:"},{"line_number":181,"context_line":"                if fixed_ip in netaddr.IPNetwork(s[\u0027cidr\u0027]):"},{"line_number":182,"context_line":"                    return s[\u0027gateway_ip\u0027]"}],"source_content_type":"text/x-python","patch_set":13,"id":"5a710552_c70cef44","line":179,"updated":"2015-12-15 05:35:37.000000000","message":"Use \"self.router.get(l3_constants.INTERFACE_KEY, [])\" would be better here.","commit_id":"30cfbb0f1c7812ae380bed99f5f7bad91d6ef7b5"},{"author":{"_account_id":13667,"name":"ZongKai LI","email":"onionpiece@163.com","username":"lzklibj"},"change_message_id":"54980d4d6ef21fe4f9aef5345c09454287a0e6b1","unresolved":false,"context_lines":[{"line_number":176,"context_line":""},{"line_number":177,"context_line":"    def get_fip_route_nexthop(self, fip_fixed_ip):"},{"line_number":178,"context_line":"        fixed_ip \u003d netaddr.IPAddress(fip_fixed_ip)"},{"line_number":179,"context_line":"        for intf in self.router[\u0027_interfaces\u0027]:"},{"line_number":180,"context_line":"            for s in intf[\u0027subnets\u0027]:"},{"line_number":181,"context_line":"                if fixed_ip in netaddr.IPNetwork(s[\u0027cidr\u0027]):"},{"line_number":182,"context_line":"                    return s[\u0027gateway_ip\u0027]"}],"source_content_type":"text/x-python","patch_set":13,"id":"5a710552_fab0998d","line":179,"in_reply_to":"5a710552_c70cef44","updated":"2015-12-15 09:12:16.000000000","message":"Sure.","commit_id":"30cfbb0f1c7812ae380bed99f5f7bad91d6ef7b5"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"ba6476fee3d8c378196e3625b74356fbd2e70b97","unresolved":false,"context_lines":[{"line_number":196,"context_line":"        return set([r.rule.split()[-1] for r in float_rules])"},{"line_number":197,"context_line":""},{"line_number":198,"context_line":"    def process_floating_ip_addresses(self, interface_name,"},{"line_number":199,"context_line":"                                      existing_fips\u003dNone):"},{"line_number":200,"context_line":"        \"\"\"Configure new added and cleanup removed floating IP addresses\"\"\""},{"line_number":201,"context_line":""},{"line_number":202,"context_line":"        fip_statuses \u003d {}"}],"source_content_type":"text/x-python","patch_set":13,"id":"5a710552_5d1d3072","line":199,"updated":"2015-12-15 05:35:37.000000000","message":"I think you don\u0027t have to add \"existing_fips\" to the parameter list. \n\nexisting_fips is always coming from \"self.get_existing_fips\". So you can just get it at line 209.\n\nThis can save some lines of change, which might be helpful in merge and cherrypick(if any)","commit_id":"30cfbb0f1c7812ae380bed99f5f7bad91d6ef7b5"},{"author":{"_account_id":13667,"name":"ZongKai LI","email":"onionpiece@163.com","username":"lzklibj"},"change_message_id":"54980d4d6ef21fe4f9aef5345c09454287a0e6b1","unresolved":false,"context_lines":[{"line_number":196,"context_line":"        return set([r.rule.split()[-1] for r in float_rules])"},{"line_number":197,"context_line":""},{"line_number":198,"context_line":"    def process_floating_ip_addresses(self, interface_name,"},{"line_number":199,"context_line":"                                      existing_fips\u003dNone):"},{"line_number":200,"context_line":"        \"\"\"Configure new added and cleanup removed floating IP addresses\"\"\""},{"line_number":201,"context_line":""},{"line_number":202,"context_line":"        fip_statuses \u003d {}"}],"source_content_type":"text/x-python","patch_set":13,"id":"5a710552_e07468fe","line":199,"in_reply_to":"5a710552_5d1d3072","updated":"2015-12-15 09:12:16.000000000","message":"Previous version is getting existing floatingip addresses from certain devices, so no matter iptable rules for floatingip are applied or stored by iptables_manager, there is no processing flow conflict there.\n\nHowever, now we are trying to get existing floatingip addresses by iptables/iptables_manager, this will both work for DVR and non-DVR router. But this will depend on processing flow, please go check L646-L648. And for why it should be optional, please go check method external_gateway_removed in dvr_local_router.py .\n\nConsider it\u0027s easy to confuse people here, I will try to update the processing flow to make things easier to understand.","commit_id":"30cfbb0f1c7812ae380bed99f5f7bad91d6ef7b5"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"844b21c6512a20c9ca31dea068ef9c27831a0b79","unresolved":false,"context_lines":[{"line_number":196,"context_line":"        return set([r.rule.split()[-1] for r in float_rules])"},{"line_number":197,"context_line":""},{"line_number":198,"context_line":"    def process_floating_ip_addresses(self, interface_name,"},{"line_number":199,"context_line":"                                      configured_fips\u003dNone):"},{"line_number":200,"context_line":"        \"\"\"Configure new added and cleanup removed floating IP addresses\"\"\""},{"line_number":201,"context_line":""},{"line_number":202,"context_line":"        fip_statuses \u003d {}"}],"source_content_type":"text/x-python","patch_set":17,"id":"1a7b0d38_668d0131","line":199,"range":{"start_line":199,"start_character":38,"end_line":199,"end_character":58},"updated":"2015-12-23 02:20:36.000000000","message":"Prefer not to have a default argument.  Just require the caller to supply configured_fips.","commit_id":"0ea97018d1c136c07c3b37e7c15812b52d7b3839"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"844b21c6512a20c9ca31dea068ef9c27831a0b79","unresolved":false,"context_lines":[{"line_number":206,"context_line":"            return fip_statuses"},{"line_number":207,"context_line":""},{"line_number":208,"context_line":"        if configured_fips is None:"},{"line_number":209,"context_line":"            configured_fips \u003d self.get_existing_fips()"},{"line_number":210,"context_line":"        new_fips \u003d set()"},{"line_number":211,"context_line":""},{"line_number":212,"context_line":"        floating_ips \u003d self.get_floating_ips()"}],"source_content_type":"text/x-python","patch_set":17,"id":"1a7b0d38_e679f130","line":209,"range":{"start_line":209,"start_character":12,"end_line":209,"end_character":54},"updated":"2015-12-23 02:20:36.000000000","message":"Move this to the caller.","commit_id":"0ea97018d1c136c07c3b37e7c15812b52d7b3839"},{"author":{"_account_id":6876,"name":"stephen-ma","email":"stephenbma43@gmail.com","username":"stephen-ma"},"change_message_id":"3ddd33e87f41959bc609d1401c401bbedb98762c","unresolved":false,"context_lines":[{"line_number":575,"context_line":"        gw_port \u003d self._router.get(\u0027gw_port\u0027)"},{"line_number":576,"context_line":"        self._handle_router_snat_rules(gw_port, interface_name)"},{"line_number":577,"context_line":""},{"line_number":578,"context_line":"    def external_gateway_nat_postroute_rules(self, interface_name):"},{"line_number":579,"context_line":"        dont_snat_traffic_to_internal_ports_if_not_to_floating_ip \u003d ("},{"line_number":580,"context_line":"            \u0027POSTROUTING\u0027, \u0027! -i %(interface_name)s \u0027"},{"line_number":581,"context_line":"                           \u0027! -o %(interface_name)s -m conntrack ! \u0027"}],"source_content_type":"text/x-python","patch_set":20,"id":"da6ed579_a5b715d7","line":578,"range":{"start_line":578,"start_character":0,"end_line":578,"end_character":67},"updated":"2016-01-13 16:11:35.000000000","message":"nit: it is better to change \u0027postroute\u0027 to \u0027postrouting\u0027","commit_id":"4f876a6c35deff37390c62a7aa8610fa3ebb04a1"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"4f4745b50c414438d1186929d4ba7ed631dbd6c4","unresolved":false,"context_lines":[{"line_number":168,"context_line":"            raise n_exc.FloatingIpSetupException("},{"line_number":169,"context_line":"                \u0027L3 agent failure to setup NAT for floating IPs\u0027)"},{"line_number":170,"context_line":""},{"line_number":171,"context_line":"    def get_fip_route_nexthop(self, fip_fixed_ip):"},{"line_number":172,"context_line":"        fixed_ip \u003d netaddr.IPAddress(fip_fixed_ip)"},{"line_number":173,"context_line":"        for intf in self.router[\u0027_interfaces\u0027]:"},{"line_number":174,"context_line":"            for s in intf[\u0027subnets\u0027]:"}],"source_content_type":"text/x-python","patch_set":23,"id":"9a68dd71_4ca9efde","line":171,"range":{"start_line":171,"start_character":8,"end_line":171,"end_character":29},"updated":"2016-01-26 17:08:05.000000000","message":"So, if I were to summarize this, it returns the gateway address from the router interface connected to the subnet containing the fixed ip.","commit_id":"a716d93c9d387141700c0790b1b0bb3f7843dc62"},{"author":{"_account_id":13667,"name":"ZongKai LI","email":"onionpiece@163.com","username":"lzklibj"},"change_message_id":"eca0124319feccd06ab0fc6872d9c6d50f7ae28d","unresolved":false,"context_lines":[{"line_number":168,"context_line":"            raise n_exc.FloatingIpSetupException("},{"line_number":169,"context_line":"                \u0027L3 agent failure to setup NAT for floating IPs\u0027)"},{"line_number":170,"context_line":""},{"line_number":171,"context_line":"    def get_fip_route_nexthop(self, fip_fixed_ip):"},{"line_number":172,"context_line":"        fixed_ip \u003d netaddr.IPAddress(fip_fixed_ip)"},{"line_number":173,"context_line":"        for intf in self.router[\u0027_interfaces\u0027]:"},{"line_number":174,"context_line":"            for s in intf[\u0027subnets\u0027]:"}],"source_content_type":"text/x-python","patch_set":23,"id":"7a5de9d1_8a3db9c5","line":171,"range":{"start_line":171,"start_character":8,"end_line":171,"end_character":29},"in_reply_to":"9a68dd71_4ca9efde","updated":"2016-02-02 05:56:54.000000000","message":"Done. I added a docstring to explain this.","commit_id":"a716d93c9d387141700c0790b1b0bb3f7843dc62"}],"neutron/agent/linux/ip_lib.py":[{"author":{"_account_id":6876,"name":"stephen-ma","email":"stephenbma43@gmail.com","username":"stephen-ma"},"change_message_id":"d9c7f2c8f28a60902edb7e181c9d0fd448271248","unresolved":false,"context_lines":[{"line_number":1055,"context_line":"    if action \u003d\u003d \u0027remove\u0027:"},{"line_number":1056,"context_line":"        arp_cmd \u003d [\u0027arp\u0027, \u0027-i\u0027, iface_name, \u0027-d\u0027, \u0027\u0027, \u0027pub\u0027]"},{"line_number":1057,"context_line":"    else:"},{"line_number":1058,"context_line":"        arp_cmd \u003d [\u0027arp\u0027, \u0027-i\u0027, iface_name, \u0027-Ds\u0027, \u0027\u0027, iface_name, \u0027pub\u0027]"},{"line_number":1059,"context_line":"    for ip in ips:"},{"line_number":1060,"context_line":"        try:"},{"line_number":1061,"context_line":"            arp_cmd[4] \u003d ip"}],"source_content_type":"text/x-python","patch_set":12,"id":"7a740942_249b2274","line":1058,"updated":"2015-12-10 22:38:49.000000000","message":"Should error out if action is not in [\u0027add\u0027, \u0027remove\u0027].","commit_id":"263a13ae4fc8f6ed729664a340b41ea6f8ce6370"},{"author":{"_account_id":13667,"name":"ZongKai LI","email":"onionpiece@163.com","username":"lzklibj"},"change_message_id":"683b7d967018aa51bce51768e5c01f5216494669","unresolved":false,"context_lines":[{"line_number":1055,"context_line":"    if action \u003d\u003d \u0027remove\u0027:"},{"line_number":1056,"context_line":"        arp_cmd \u003d [\u0027arp\u0027, \u0027-i\u0027, iface_name, \u0027-d\u0027, \u0027\u0027, \u0027pub\u0027]"},{"line_number":1057,"context_line":"    else:"},{"line_number":1058,"context_line":"        arp_cmd \u003d [\u0027arp\u0027, \u0027-i\u0027, iface_name, \u0027-Ds\u0027, \u0027\u0027, iface_name, \u0027pub\u0027]"},{"line_number":1059,"context_line":"    for ip in ips:"},{"line_number":1060,"context_line":"        try:"},{"line_number":1061,"context_line":"            arp_cmd[4] \u003d ip"}],"source_content_type":"text/x-python","patch_set":12,"id":"7a740942_42a6f7ae","line":1058,"in_reply_to":"7a740942_249b2274","updated":"2015-12-14 09:03:18.000000000","message":"Not sure that, I think that should be code contributor\u0027s responsibility.","commit_id":"263a13ae4fc8f6ed729664a340b41ea6f8ce6370"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"ba6476fee3d8c378196e3625b74356fbd2e70b97","unresolved":false,"context_lines":[{"line_number":1053,"context_line":"def update_arp_proxies(ns_name, ips, iface_name, action):"},{"line_number":1054,"context_line":"    ip_wrapper \u003d IPWrapper(namespace\u003dns_name)"},{"line_number":1055,"context_line":"    if action \u003d\u003d \u0027remove\u0027:"},{"line_number":1056,"context_line":"        arp_cmd \u003d [\u0027arp\u0027, \u0027-i\u0027, iface_name, \u0027-d\u0027, \u0027\u0027, \u0027pub\u0027]"},{"line_number":1057,"context_line":"    else:"},{"line_number":1058,"context_line":"        arp_cmd \u003d [\u0027arp\u0027, \u0027-i\u0027, iface_name, \u0027-Ds\u0027, \u0027\u0027, iface_name, \u0027pub\u0027]"},{"line_number":1059,"context_line":"    for ip in ips:"}],"source_content_type":"text/x-python","patch_set":13,"id":"5a710552_f8aaea84","line":1056,"updated":"2015-12-15 05:35:37.000000000","message":"As the code here is partially duplicated with code above, you could try something else for example:\n\ngetattr(sys.modules[__name__], \"%s_arp_proxy\" % action)(ns_name, ips, iface_name)","commit_id":"30cfbb0f1c7812ae380bed99f5f7bad91d6ef7b5"},{"author":{"_account_id":13667,"name":"ZongKai LI","email":"onionpiece@163.com","username":"lzklibj"},"change_message_id":"54980d4d6ef21fe4f9aef5345c09454287a0e6b1","unresolved":false,"context_lines":[{"line_number":1053,"context_line":"def update_arp_proxies(ns_name, ips, iface_name, action):"},{"line_number":1054,"context_line":"    ip_wrapper \u003d IPWrapper(namespace\u003dns_name)"},{"line_number":1055,"context_line":"    if action \u003d\u003d \u0027remove\u0027:"},{"line_number":1056,"context_line":"        arp_cmd \u003d [\u0027arp\u0027, \u0027-i\u0027, iface_name, \u0027-d\u0027, \u0027\u0027, \u0027pub\u0027]"},{"line_number":1057,"context_line":"    else:"},{"line_number":1058,"context_line":"        arp_cmd \u003d [\u0027arp\u0027, \u0027-i\u0027, iface_name, \u0027-Ds\u0027, \u0027\u0027, iface_name, \u0027pub\u0027]"},{"line_number":1059,"context_line":"    for ip in ips:"}],"source_content_type":"text/x-python","patch_set":13,"id":"5a710552_a09ae012","line":1056,"in_reply_to":"5a710552_f8aaea84","updated":"2015-12-15 09:12:16.000000000","message":"OK, I will try to do something here.","commit_id":"30cfbb0f1c7812ae380bed99f5f7bad91d6ef7b5"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"844b21c6512a20c9ca31dea068ef9c27831a0b79","unresolved":false,"context_lines":[{"line_number":308,"context_line":"            A netaddr.IPAddress or netaddr.Network representing the IP address"},{"line_number":309,"context_line":"            can also be passed."},{"line_number":310,"context_line":"        \"\"\""},{"line_number":311,"context_line":"        self.addr.delete(cidr)"},{"line_number":312,"context_line":"        ip_str \u003d str(netaddr.IPNetwork(cidr).ip)"},{"line_number":313,"context_line":"        ip_wrapper \u003d IPWrapper(namespace\u003dself.namespace)"},{"line_number":314,"context_line":"        ip_wrapper.delete_conntrack_state(ip_str)"}],"source_content_type":"text/x-python","patch_set":17,"id":"1a7b0d38_c66dd533","line":311,"range":{"start_line":311,"start_character":30,"end_line":311,"end_character":30},"updated":"2015-12-23 02:20:36.000000000","message":"nit:  Put this line back.","commit_id":"0ea97018d1c136c07c3b37e7c15812b52d7b3839"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"844b21c6512a20c9ca31dea068ef9c27831a0b79","unresolved":false,"context_lines":[{"line_number":1040,"context_line":"    return \u0027%s/64\u0027 % netaddr.EUI(mac_addr).ipv6_link_local()"},{"line_number":1041,"context_line":""},{"line_number":1042,"context_line":""},{"line_number":1043,"context_line":"def update_arp_proxies(ns_name, ips, iface_name, action):"},{"line_number":1044,"context_line":"    ip_wrapper \u003d IPWrapper(namespace\u003dns_name)"},{"line_number":1045,"context_line":"    if action \u003d\u003d \u0027remove\u0027:"},{"line_number":1046,"context_line":"        arp_cmd \u003d [\u0027arp\u0027, \u0027-i\u0027, iface_name, \u0027-d\u0027, \u0027\u0027, \u0027pub\u0027]"}],"source_content_type":"text/x-python","patch_set":17,"id":"1a7b0d38_a9c17a30","line":1043,"range":{"start_line":1043,"start_character":4,"end_line":1043,"end_character":22},"updated":"2015-12-23 02:20:36.000000000","message":"Honestly, I think this method has much less common code than it is trying to make common.  Just write add and remove separately, the cost of sharing code here is an awkward and confusing common method.\n\ndef add_arp_proxies(ns_name, ips, iface_name):\n    ip_wrapper \u003d IPWrapper(namespace\u003dns_name)\n    for ip in ips:\n        arp_cmd \u003d [\u0027arp\u0027, \u0027-i\u0027, iface_name, \u0027-Ds\u0027, ip, iface_name, \u0027pub\u0027]\n        try:\n            ip_wrapper.netns.execute(arp_cmd, check_exit_code\u003dTrue)\n        except Exception as ex:\n            ...\n\n\ndef remove_arp_proxies(ns_name, ips, iface_name):\n    ip_wrapper \u003d IPWrapper(namespace\u003dns_name)\n    for ip in ips:\n        arp_cmd \u003d [\u0027arp\u0027, \u0027-i\u0027, iface_name, \u0027-d\u0027, ip, \u0027pub\u0027]\n        try:\n            ip_wrapper.netns.execute(arp_cmd, check_exit_code\u003dTrue)\n        except Exception as ex:\n            no_arp_proxy \u003d (isinstance(ex, RuntimeError) and\n                            \u0027No ARP entry\u0027 in str(ex))\n            if not no_arp_proxy:\n                msg \u003d _LE(\"Failed to clean stale ARP entry for ip %(ip)s on\"\n                          \"%(iface)s in namespace %(ns)s\")\n                LOG.exception(msg, {\u0027ip\u0027: ip,\n                                    \u0027iface\u0027: iface_name,\n                                    \u0027ns\u0027: ns_name})","commit_id":"0ea97018d1c136c07c3b37e7c15812b52d7b3839"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"844b21c6512a20c9ca31dea068ef9c27831a0b79","unresolved":false,"context_lines":[{"line_number":1042,"context_line":""},{"line_number":1043,"context_line":"def update_arp_proxies(ns_name, ips, iface_name, action):"},{"line_number":1044,"context_line":"    ip_wrapper \u003d IPWrapper(namespace\u003dns_name)"},{"line_number":1045,"context_line":"    if action \u003d\u003d \u0027remove\u0027:"},{"line_number":1046,"context_line":"        arp_cmd \u003d [\u0027arp\u0027, \u0027-i\u0027, iface_name, \u0027-d\u0027, \u0027\u0027, \u0027pub\u0027]"},{"line_number":1047,"context_line":"    else:"},{"line_number":1048,"context_line":"        arp_cmd \u003d [\u0027arp\u0027, \u0027-i\u0027, iface_name, \u0027-Ds\u0027, \u0027\u0027, iface_name, \u0027pub\u0027]"}],"source_content_type":"text/x-python","patch_set":17,"id":"1a7b0d38_4663251b","line":1045,"range":{"start_line":1045,"start_character":4,"end_line":1045,"end_character":25},"updated":"2015-12-23 02:20:36.000000000","message":"Please don\u0027t use this pattern passing action to a common function.  Just move the setting of arp_cmd to add_arp_proxy and remove_arp_proxy and pass it in to this function.","commit_id":"0ea97018d1c136c07c3b37e7c15812b52d7b3839"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"844b21c6512a20c9ca31dea068ef9c27831a0b79","unresolved":false,"context_lines":[{"line_number":1046,"context_line":"        arp_cmd \u003d [\u0027arp\u0027, \u0027-i\u0027, iface_name, \u0027-d\u0027, \u0027\u0027, \u0027pub\u0027]"},{"line_number":1047,"context_line":"    else:"},{"line_number":1048,"context_line":"        arp_cmd \u003d [\u0027arp\u0027, \u0027-i\u0027, iface_name, \u0027-Ds\u0027, \u0027\u0027, iface_name, \u0027pub\u0027]"},{"line_number":1049,"context_line":"    for ip in ips:"},{"line_number":1050,"context_line":"        try:"},{"line_number":1051,"context_line":"            arp_cmd[4] \u003d ip"},{"line_number":1052,"context_line":"            ip_wrapper.netns.execute(arp_cmd, check_exit_code\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":17,"id":"1a7b0d38_a98dfa7c","line":1049,"range":{"start_line":1049,"start_character":4,"end_line":1049,"end_character":17},"updated":"2015-12-23 02:20:36.000000000","message":"Move the iteration out to the callers.","commit_id":"0ea97018d1c136c07c3b37e7c15812b52d7b3839"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"844b21c6512a20c9ca31dea068ef9c27831a0b79","unresolved":false,"context_lines":[{"line_number":1048,"context_line":"        arp_cmd \u003d [\u0027arp\u0027, \u0027-i\u0027, iface_name, \u0027-Ds\u0027, \u0027\u0027, iface_name, \u0027pub\u0027]"},{"line_number":1049,"context_line":"    for ip in ips:"},{"line_number":1050,"context_line":"        try:"},{"line_number":1051,"context_line":"            arp_cmd[4] \u003d ip"},{"line_number":1052,"context_line":"            ip_wrapper.netns.execute(arp_cmd, check_exit_code\u003dTrue)"},{"line_number":1053,"context_line":"        except Exception as ex:"},{"line_number":1054,"context_line":"            no_arp_proxy \u003d (isinstance(ex, RuntimeError) and"}],"source_content_type":"text/x-python","patch_set":17,"id":"1a7b0d38_a9009aab","line":1051,"range":{"start_line":1051,"start_character":12,"end_line":1051,"end_character":19},"updated":"2015-12-23 02:20:36.000000000","message":"This depends on the ip being the 5th argument in both commands.  I think you got lucky there but I think doing this is fragile.  I would go for a more straight-forward approach that isn\u0027t brittle.","commit_id":"0ea97018d1c136c07c3b37e7c15812b52d7b3839"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"844b21c6512a20c9ca31dea068ef9c27831a0b79","unresolved":false,"context_lines":[{"line_number":1051,"context_line":"            arp_cmd[4] \u003d ip"},{"line_number":1052,"context_line":"            ip_wrapper.netns.execute(arp_cmd, check_exit_code\u003dTrue)"},{"line_number":1053,"context_line":"        except Exception as ex:"},{"line_number":1054,"context_line":"            no_arp_proxy \u003d (isinstance(ex, RuntimeError) and"},{"line_number":1055,"context_line":"                            \u0027No ARP entry\u0027 in str(ex))"},{"line_number":1056,"context_line":"            if not no_arp_proxy:"},{"line_number":1057,"context_line":"                msg \u003d _LE(\"Failed to clean stale ARP entry for ip %(ip)s on\""}],"source_content_type":"text/x-python","patch_set":17,"id":"1a7b0d38_698dd230","line":1054,"updated":"2015-12-23 02:20:36.000000000","message":"Is this exception handling code relevant to both add and remove contexts?  The way it is written, it seems more relevant to the remove context.","commit_id":"0ea97018d1c136c07c3b37e7c15812b52d7b3839"},{"author":{"_account_id":13667,"name":"ZongKai LI","email":"onionpiece@163.com","username":"lzklibj"},"change_message_id":"40e5db4d012ccb9d9bd90ddbce0081ac3d1aad80","unresolved":false,"context_lines":[{"line_number":1051,"context_line":"            arp_cmd[4] \u003d ip"},{"line_number":1052,"context_line":"            ip_wrapper.netns.execute(arp_cmd, check_exit_code\u003dTrue)"},{"line_number":1053,"context_line":"        except Exception as ex:"},{"line_number":1054,"context_line":"            no_arp_proxy \u003d (isinstance(ex, RuntimeError) and"},{"line_number":1055,"context_line":"                            \u0027No ARP entry\u0027 in str(ex))"},{"line_number":1056,"context_line":"            if not no_arp_proxy:"},{"line_number":1057,"context_line":"                msg \u003d _LE(\"Failed to clean stale ARP entry for ip %(ip)s on\""}],"source_content_type":"text/x-python","patch_set":17,"id":"1a7b0d38_7be8197d","line":1054,"in_reply_to":"1a7b0d38_698dd230","updated":"2015-12-23 16:31:26.000000000","message":"Yes, it should only related to removal action, I will update this based on your comments. Thanks. :)","commit_id":"0ea97018d1c136c07c3b37e7c15812b52d7b3839"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"844b21c6512a20c9ca31dea068ef9c27831a0b79","unresolved":false,"context_lines":[{"line_number":1054,"context_line":"            no_arp_proxy \u003d (isinstance(ex, RuntimeError) and"},{"line_number":1055,"context_line":"                            \u0027No ARP entry\u0027 in str(ex))"},{"line_number":1056,"context_line":"            if not no_arp_proxy:"},{"line_number":1057,"context_line":"                msg \u003d _LE(\"Failed to clean stale ARP entry for ip %(ip)s on\""},{"line_number":1058,"context_line":"                          \"%(iface)s in namespace %(ns)s\")"},{"line_number":1059,"context_line":"                LOG.exception(msg, {\u0027ip\u0027: ip,"},{"line_number":1060,"context_line":"                                    \u0027iface\u0027: iface_name,"}],"source_content_type":"text/x-python","patch_set":17,"id":"1a7b0d38_69bf12d1","line":1057,"range":{"start_line":1057,"start_character":27,"end_line":1057,"end_character":75},"updated":"2015-12-23 02:20:36.000000000","message":"This message isn\u0027t relevant in an \u0027add\u0027 context.","commit_id":"0ea97018d1c136c07c3b37e7c15812b52d7b3839"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"844b21c6512a20c9ca31dea068ef9c27831a0b79","unresolved":false,"context_lines":[{"line_number":1061,"context_line":"                                    \u0027ns\u0027: ns_name})"},{"line_number":1062,"context_line":""},{"line_number":1063,"context_line":""},{"line_number":1064,"context_line":"def add_arp_proxy(ns_name, ip, iface_name):"},{"line_number":1065,"context_line":"    update_arp_proxies(ns_name, [ip], iface_name, \u0027add\u0027)"},{"line_number":1066,"context_line":""},{"line_number":1067,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"1a7b0d38_c96886bd","line":1064,"range":{"start_line":1064,"start_character":4,"end_line":1064,"end_character":17},"updated":"2015-12-23 02:20:36.000000000","message":"I would add a new method call add_arp_proxies that accepts an array of ips to be called from ha_router.py and does the iteration.","commit_id":"0ea97018d1c136c07c3b37e7c15812b52d7b3839"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"844b21c6512a20c9ca31dea068ef9c27831a0b79","unresolved":false,"context_lines":[{"line_number":1065,"context_line":"    update_arp_proxies(ns_name, [ip], iface_name, \u0027add\u0027)"},{"line_number":1066,"context_line":""},{"line_number":1067,"context_line":""},{"line_number":1068,"context_line":"def remove_arp_proxy(ns_name, ip, iface_name):"},{"line_number":1069,"context_line":"    update_arp_proxies(ns_name, [ip], iface_name, \u0027remove\u0027)"}],"source_content_type":"text/x-python","patch_set":17,"id":"1a7b0d38_a95f3a0d","line":1068,"range":{"start_line":1068,"start_character":4,"end_line":1068,"end_character":20},"updated":"2015-12-23 02:20:36.000000000","message":"ditto","commit_id":"0ea97018d1c136c07c3b37e7c15812b52d7b3839"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"56132160b7986f887b74aa0d8faf8fb8f929af98","unresolved":false,"context_lines":[{"line_number":257,"context_line":"                               extra_ok_codes\u003d[1])"},{"line_number":258,"context_line":""},{"line_number":259,"context_line":"        except RuntimeError:"},{"line_number":260,"context_line":"            LOG.exception(_LE(\"Failed deleting ingress connection state of\""},{"line_number":261,"context_line":"                              \" floatingip %s\"), ip)"},{"line_number":262,"context_line":""},{"line_number":263,"context_line":"        # Delete conntrack state for egress traffic"}],"source_content_type":"text/x-python","patch_set":20,"id":"da6ed579_67adaa30","line":260,"updated":"2016-01-13 14:03:06.000000000","message":"why spaces at the beginning of the line?","commit_id":"4f876a6c35deff37390c62a7aa8610fa3ebb04a1"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"56132160b7986f887b74aa0d8faf8fb8f929af98","unresolved":false,"context_lines":[{"line_number":258,"context_line":""},{"line_number":259,"context_line":"        except RuntimeError:"},{"line_number":260,"context_line":"            LOG.exception(_LE(\"Failed deleting ingress connection state of\""},{"line_number":261,"context_line":"                              \" floatingip %s\"), ip)"},{"line_number":262,"context_line":""},{"line_number":263,"context_line":"        # Delete conntrack state for egress traffic"},{"line_number":264,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":20,"id":"da6ed579_6774cace","line":261,"updated":"2016-01-13 14:03:06.000000000","message":"why do we need to log the exception trace here and not just the exception?","commit_id":"4f876a6c35deff37390c62a7aa8610fa3ebb04a1"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"56132160b7986f887b74aa0d8faf8fb8f929af98","unresolved":false,"context_lines":[{"line_number":266,"context_line":"                               check_exit_code\u003dTrue,"},{"line_number":267,"context_line":"                               extra_ok_codes\u003d[1])"},{"line_number":268,"context_line":"        except RuntimeError:"},{"line_number":269,"context_line":"            LOG.exception(_LE(\"Failed deleting egress connection state of\""},{"line_number":270,"context_line":"                              \" floatingip %s\"), ip)"},{"line_number":271,"context_line":""},{"line_number":272,"context_line":""}],"source_content_type":"text/x-python","patch_set":20,"id":"da6ed579_e7675a23","line":269,"updated":"2016-01-13 14:03:06.000000000","message":"ditto","commit_id":"4f876a6c35deff37390c62a7aa8610fa3ebb04a1"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"56132160b7986f887b74aa0d8faf8fb8f929af98","unresolved":false,"context_lines":[{"line_number":1048,"context_line":"            arp_cmd \u003d [\u0027arp\u0027, \u0027-i\u0027, iface_name, \u0027-Ds\u0027, ip, iface_name, \u0027pub\u0027]"},{"line_number":1049,"context_line":"            ip_wrapper.netns.execute(arp_cmd, check_exit_code\u003dTrue)"},{"line_number":1050,"context_line":"        except Exception:"},{"line_number":1051,"context_line":"            msg \u003d _LE(\"Failed to add ARP entry for ip %(ip)s on\""},{"line_number":1052,"context_line":"                      \"%(iface)s in namespace %(ns)s\")"},{"line_number":1053,"context_line":"            LOG.exception(msg, {\u0027ip\u0027: ip,"},{"line_number":1054,"context_line":"                                \u0027iface\u0027: iface_name,"}],"source_content_type":"text/x-python","patch_set":20,"id":"da6ed579_87453e71","line":1051,"updated":"2016-01-13 14:03:06.000000000","message":"missing space at the end of the line","commit_id":"4f876a6c35deff37390c62a7aa8610fa3ebb04a1"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"56132160b7986f887b74aa0d8faf8fb8f929af98","unresolved":false,"context_lines":[{"line_number":1065,"context_line":"            no_arp_proxy \u003d (isinstance(ex, RuntimeError) and"},{"line_number":1066,"context_line":"                            \u0027No ARP entry\u0027 in str(ex))"},{"line_number":1067,"context_line":"            if not no_arp_proxy:"},{"line_number":1068,"context_line":"                msg \u003d _LE(\"Failed to remove ARP entry for ip %(ip)s on\""},{"line_number":1069,"context_line":"                          \"%(iface)s in namespace %(ns)s\")"},{"line_number":1070,"context_line":"                LOG.exception(msg, {\u0027ip\u0027: ip,"},{"line_number":1071,"context_line":"                                    \u0027iface\u0027: iface_name,"}],"source_content_type":"text/x-python","patch_set":20,"id":"da6ed579_075aae4f","line":1068,"updated":"2016-01-13 14:03:06.000000000","message":"ditto","commit_id":"4f876a6c35deff37390c62a7aa8610fa3ebb04a1"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"4f4745b50c414438d1186929d4ba7ed631dbd6c4","unresolved":false,"context_lines":[{"line_number":309,"context_line":"            A netaddr.IPAddress or netaddr.Network representing the IP address"},{"line_number":310,"context_line":"            can also be passed."},{"line_number":311,"context_line":"        \"\"\""},{"line_number":312,"context_line":""},{"line_number":313,"context_line":"        self.addr.delete(cidr)"},{"line_number":314,"context_line":"        ip_str \u003d str(netaddr.IPNetwork(cidr).ip)"},{"line_number":315,"context_line":"        ip_wrapper \u003d IPWrapper(namespace\u003dself.namespace)"}],"source_content_type":"text/x-python","patch_set":23,"id":"9a68dd71_0c1d370f","line":312,"updated":"2016-01-26 17:08:05.000000000","message":"Revert this whitespace change.","commit_id":"a716d93c9d387141700c0790b1b0bb3f7843dc62"},{"author":{"_account_id":13667,"name":"ZongKai LI","email":"onionpiece@163.com","username":"lzklibj"},"change_message_id":"eca0124319feccd06ab0fc6872d9c6d50f7ae28d","unresolved":false,"context_lines":[{"line_number":309,"context_line":"            A netaddr.IPAddress or netaddr.Network representing the IP address"},{"line_number":310,"context_line":"            can also be passed."},{"line_number":311,"context_line":"        \"\"\""},{"line_number":312,"context_line":""},{"line_number":313,"context_line":"        self.addr.delete(cidr)"},{"line_number":314,"context_line":"        ip_str \u003d str(netaddr.IPNetwork(cidr).ip)"},{"line_number":315,"context_line":"        ip_wrapper \u003d IPWrapper(namespace\u003dself.namespace)"}],"source_content_type":"text/x-python","patch_set":23,"id":"7a5de9d1_ff1af17b","line":312,"in_reply_to":"9a68dd71_0c1d370f","updated":"2016-02-02 05:56:54.000000000","message":"Done","commit_id":"a716d93c9d387141700c0790b1b0bb3f7843dc62"}],"neutron/agent/linux/keepalived.py":[{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"805319a74e8e87be0dba34684ce45c36e6aedbe3","unresolved":false,"context_lines":[{"line_number":155,"context_line":"    def routes(self):"},{"line_number":156,"context_line":"        if six.PY3:"},{"line_number":157,"context_line":"            return (self.gateway_routes + self.extra_routes +"},{"line_number":158,"context_line":"                    self.extra_subnets + list(self.fip_routes.values()))"},{"line_number":159,"context_line":"        else:"},{"line_number":160,"context_line":"            return (self.gateway_routes + self.extra_routes +"},{"line_number":161,"context_line":"                    self.extra_subnets + self.fip_routes.values())"}],"source_content_type":"text/x-python","patch_set":10,"id":"ba8a016a_d6f99bc9","line":158,"updated":"2015-11-24 05:12:26.000000000","message":"You could just use list(self.fip_routes.values()) here, it works for py2.\n\nActually, I think to keep fip_routes as a list would be better. It will keep consistence, and it will not prevent you from implementing any functionality.","commit_id":"af71c5f18c92f810f650f37a23278b9248f77945"},{"author":{"_account_id":13667,"name":"ZongKai LI","email":"onionpiece@163.com","username":"lzklibj"},"change_message_id":"8dd546c57fff5b8b2dee95cd195cfe661f4ff085","unresolved":false,"context_lines":[{"line_number":155,"context_line":"    def routes(self):"},{"line_number":156,"context_line":"        if six.PY3:"},{"line_number":157,"context_line":"            return (self.gateway_routes + self.extra_routes +"},{"line_number":158,"context_line":"                    self.extra_subnets + list(self.fip_routes.values()))"},{"line_number":159,"context_line":"        else:"},{"line_number":160,"context_line":"            return (self.gateway_routes + self.extra_routes +"},{"line_number":161,"context_line":"                    self.extra_subnets + self.fip_routes.values())"}],"source_content_type":"text/x-python","patch_set":10,"id":"ba8a016a_0c44f671","line":158,"in_reply_to":"ba8a016a_d6f99bc9","updated":"2015-11-24 06:44:51.000000000","message":"Yes, that works for py2, but seems not no necessary.\n\nTo routes method here, it is okey to use fip_routes as list.\n\nTo get_existing_fips method in ha_router.py, it\u0027s a difference between using builtin method or using manual for-loop. Both ok to me, not a big deal.\n\nTo remove_floating_ip in ha_router.py, for KeepalivedVirtualRoute has no comparison methods, so if fip_routes is a list, we cannot directly use method like list.remove(..), we have to do a for-loop scan to remove target KeepalivedVirtualRoute object.\n\nSo I prefer to use fip_routes as a dict, will that make sense to you?","commit_id":"af71c5f18c92f810f650f37a23278b9248f77945"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"844b21c6512a20c9ca31dea068ef9c27831a0b79","unresolved":false,"context_lines":[{"line_number":148,"context_line":"                    self.extra_subnets + list(self.fip_routes.values()))"},{"line_number":149,"context_line":"        else:"},{"line_number":150,"context_line":"            return (self.gateway_routes + self.extra_routes +"},{"line_number":151,"context_line":"                    self.extra_subnets + self.fip_routes.values())"},{"line_number":152,"context_line":""},{"line_number":153,"context_line":"    def __len__(self):"},{"line_number":154,"context_line":"        return len(self.routes)"}],"source_content_type":"text/x-python","patch_set":17,"id":"1a7b0d38_29034a75","line":151,"range":{"start_line":151,"start_character":41,"end_line":151,"end_character":63},"updated":"2015-12-23 02:20:36.000000000","message":"Just wrap it with list(...) for both versions of python and get rid of all the six nonsense.","commit_id":"0ea97018d1c136c07c3b37e7c15812b52d7b3839"}],"neutron/tests/common/l3_test_common.py":[{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"844b21c6512a20c9ca31dea068ef9c27831a0b79","unresolved":false,"context_lines":[{"line_number":109,"context_line":"            \u0027port_id\u0027: _uuid(),"},{"line_number":110,"context_line":"            \u0027status\u0027: \u0027DOWN\u0027,"},{"line_number":111,"context_line":"            \u0027floating_ip_address\u0027: \u002719.4.4.2\u0027,"},{"line_number":112,"context_line":"            \u0027fixed_ip_address\u0027: \u002735.4.0.3\u0027}]"},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"    router_append_interface(router, count\u003dnum_internal_ports,"},{"line_number":115,"context_line":"                            ip_version\u003dip_version, dual_stack\u003ddual_stack)"}],"source_content_type":"text/x-python","patch_set":17,"id":"1a7b0d38_09352e45","line":112,"range":{"start_line":112,"start_character":33,"end_line":112,"end_character":41},"updated":"2015-12-23 02:20:36.000000000","message":"Curious:  why did you feel the need to change this address?","commit_id":"0ea97018d1c136c07c3b37e7c15812b52d7b3839"},{"author":{"_account_id":13667,"name":"ZongKai LI","email":"onionpiece@163.com","username":"lzklibj"},"change_message_id":"40e5db4d012ccb9d9bd90ddbce0081ac3d1aad80","unresolved":false,"context_lines":[{"line_number":109,"context_line":"            \u0027port_id\u0027: _uuid(),"},{"line_number":110,"context_line":"            \u0027status\u0027: \u0027DOWN\u0027,"},{"line_number":111,"context_line":"            \u0027floating_ip_address\u0027: \u002719.4.4.2\u0027,"},{"line_number":112,"context_line":"            \u0027fixed_ip_address\u0027: \u002735.4.0.3\u0027}]"},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"    router_append_interface(router, count\u003dnum_internal_ports,"},{"line_number":115,"context_line":"                            ip_version\u003dip_version, dual_stack\u003ddual_stack)"}],"source_content_type":"text/x-python","patch_set":17,"id":"1a7b0d38_db310da4","line":112,"range":{"start_line":112,"start_character":33,"end_line":112,"end_character":41},"in_reply_to":"1a7b0d38_09352e45","updated":"2015-12-23 16:31:26.000000000","message":"In https://review.openstack.org/#/c/246855/17/neutron/agent/l3/router_info.py , I add a method get_fip_route_nexthop .\nIt will find fixed_ip_address subnet gateway for floatingip. And for that, keep using previous fixed_ip_address \u002710.0.0.1\u0027 will cause get_fip_route_nexthop return None, and make connectivity test failure in functional test.\n\nBeside that, for IPv4, the router will have subnets with CIDR 35.4.x.0/24 as interface, you find that in L145 in this page. So previous fixed_ip_address will cause a scenario that, associating a floatingip to unreachable fixed_ip for router, I don\u0027t think that\u0027s valid.","commit_id":"0ea97018d1c136c07c3b37e7c15812b52d7b3839"}]}