)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"1b313152a0e442d940adda7186ce1320c2b337e5","unresolved":false,"context_lines":[{"line_number":14,"context_line":"packet to be 1524 bytes."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"In case the underlying infrastructure\u0027s MTU is set to 1500, the packets"},{"line_number":17,"context_line":"will drop somewhere along the way, causing problems which can be"},{"line_number":18,"context_line":"somewhat difficult to debug."},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"This patch adds a sanity check for whether or not the physical"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"9aa7fdbe_d115d2e5","line":17,"updated":"2014-10-23 17:10:06.000000000","message":"There is a chance they might, but this is not certain. What can be said for certain is that there may be a performance degradation due to the potential fragmentation of frames. I\u0027d invite you to revise the form of this commit message.","commit_id":"5a8e1519a6805a310d6f573ee1b0ac31588ab90c"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"1b69cf1d627169ec350e5dd9f242a3cb931483b0","unresolved":false,"context_lines":[{"line_number":14,"context_line":"packet to be 1524 bytes."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"In case the underlying infrastructure\u0027s MTU is set to 1500, the packets"},{"line_number":17,"context_line":"will drop somewhere along the way, causing problems which can be"},{"line_number":18,"context_line":"somewhat difficult to debug."},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"This patch adds a sanity check for whether or not the physical"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"7a8c0949_118411d3","line":17,"in_reply_to":"9aa7fdbe_d115d2e5","updated":"2014-11-03 11:56:45.000000000","message":"Done","commit_id":"5a8e1519a6805a310d6f573ee1b0ac31588ab90c"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"46e0fe8b802013488337da351396b1b4ea338390","unresolved":false,"context_lines":[{"line_number":16,"context_line":"In case the underlying infrastructure\u0027s MTU is set to 1500, there may be"},{"line_number":17,"context_line":"a performance degradation due to the potential fragmentation of frames,"},{"line_number":18,"context_line":"causing problems which can be somewhat difficult to debug."},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"This patch adds a sanity check for whether or not the physical"},{"line_number":21,"context_line":"interface\u0027s MTU is high enough (for our previous example, 1524 is"},{"line_number":22,"context_line":"needed), and if not a warning is printed. This is only a warning since"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":10,"id":"1a930d6b_54124263","line":19,"updated":"2015-01-20 21:50:50.000000000","message":"I\u0027m not sure I agree with the idea behind this patch. It doesn\u0027t \"only print a warning\", it actually causes neutron-sanity-check to return non-zero indicating an actual failure for something that may not be a failure at all since, as you say, instance MTUs can be lowered (and often are by with dhcp-option-force). This lessens the usefulness of using neutron-sanity-check in an automated fashion for people using that solution.","commit_id":"537d72c3d20571f0552927675b5c8cc8604e3d19"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"669ebfdd3bd7bbdf2daec7ce45e763903689c0b6","unresolved":false,"context_lines":[{"line_number":16,"context_line":"In case the underlying infrastructure\u0027s MTU is set to 1500, there may be"},{"line_number":17,"context_line":"a performance degradation due to the potential fragmentation of frames,"},{"line_number":18,"context_line":"causing problems which can be somewhat difficult to debug."},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"This patch adds a sanity check for whether or not the physical"},{"line_number":21,"context_line":"interface\u0027s MTU is high enough (for our previous example, 1524 is"},{"line_number":22,"context_line":"needed), and if not a warning is printed. This is only a warning since"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":10,"id":"1a930d6b_3df90d4e","line":19,"in_reply_to":"1a930d6b_54124263","updated":"2015-01-21 09:39:13.000000000","message":"I see your point. I\u0027ll modify the code to return success values when only warnings are printed, and return failure values for local_ip conflicts.","commit_id":"537d72c3d20571f0552927675b5c8cc8604e3d19"},{"author":{"_account_id":2874,"name":"yong sheng gong","email":"gong.yongsheng@99cloud.net","username":"gongysh"},"change_message_id":"9ea8cb62efb8711e110bf74548dd74d8d7c23202","unresolved":false,"context_lines":[{"line_number":11,"context_line":"current options offered), which causes the packets to increase in size."},{"line_number":12,"context_line":"In other words, when using GRE, a packet of size 1500 will be changed"},{"line_number":13,"context_line":"and the GRE header (24 bytes) will be added, causing the actual sent"},{"line_number":14,"context_line":"packet to be 1524 bytes."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"In case the underlying infrastructure\u0027s MTU is set to 1500, there may be"},{"line_number":17,"context_line":"a performance degradation due to the potential fragmentation of frames,"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":13,"id":"9a80dd14_17832905","line":14,"updated":"2015-03-05 07:51:53.000000000","message":"1542?","commit_id":"0cd5f423e927f9892ca4d6917e1f7056f07b17a2"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"f9c76baec8b3b69e19346e26a0c6ca0c04fa4df3","unresolved":false,"context_lines":[{"line_number":11,"context_line":"current options offered), which causes the packets to increase in size."},{"line_number":12,"context_line":"In other words, when using GRE, a packet of size 1500 will be changed"},{"line_number":13,"context_line":"and the GRE header (24 bytes) will be added, causing the actual sent"},{"line_number":14,"context_line":"packet to be 1524 bytes."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"In case the underlying infrastructure\u0027s MTU is set to 1500, there may be"},{"line_number":17,"context_line":"a performance degradation due to the potential fragmentation of frames,"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":13,"id":"9a80dd14_2889aceb","line":14,"in_reply_to":"9a80dd14_17832905","updated":"2015-03-05 09:04:16.000000000","message":"yep. fixed.","commit_id":"0cd5f423e927f9892ca4d6917e1f7056f07b17a2"}],"neutron/cmd/sanity/checks.py":[{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"82a828ea1de6ca5b36000c4efb0758be5b6c1818","unresolved":false,"context_lines":[{"line_number":27,"context_line":""},{"line_number":28,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"# How much overhead does each of the tunnel types force?"},{"line_number":31,"context_line":"TUNNEL_OVERHEAD \u003d {"},{"line_number":32,"context_line":"    \u0027gre\u0027: 24,"},{"line_number":33,"context_line":"    \u0027vxlan\u0027: 50,"}],"source_content_type":"text/x-python","patch_set":1,"id":"9aa7fdbe_95770902","line":30,"updated":"2014-10-21 12:10:30.000000000","message":"what do you mean? is this bytes? the comment is a bit cryptic?","commit_id":"5a8e1519a6805a310d6f573ee1b0ac31588ab90c"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"a89d711d24d06cc3a4cc1f2ab86e9073fd2f7381","unresolved":false,"context_lines":[{"line_number":27,"context_line":""},{"line_number":28,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"# How much overhead does each of the tunnel types force?"},{"line_number":31,"context_line":"TUNNEL_OVERHEAD \u003d {"},{"line_number":32,"context_line":"    \u0027gre\u0027: 24,"},{"line_number":33,"context_line":"    \u0027vxlan\u0027: 50,"}],"source_content_type":"text/x-python","patch_set":1,"id":"9aa7fdbe_7721a44c","line":30,"in_reply_to":"9aa7fdbe_1f75e197","updated":"2014-10-22 12:40:34.000000000","message":"Will fix.","commit_id":"5a8e1519a6805a310d6f573ee1b0ac31588ab90c"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"1b69cf1d627169ec350e5dd9f242a3cb931483b0","unresolved":false,"context_lines":[{"line_number":27,"context_line":""},{"line_number":28,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"# How much overhead does each of the tunnel types force?"},{"line_number":31,"context_line":"TUNNEL_OVERHEAD \u003d {"},{"line_number":32,"context_line":"    \u0027gre\u0027: 24,"},{"line_number":33,"context_line":"    \u0027vxlan\u0027: 50,"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a8c0949_91c48141","line":30,"in_reply_to":"9aa7fdbe_7721a44c","updated":"2014-11-03 11:56:45.000000000","message":"Done","commit_id":"5a8e1519a6805a310d6f573ee1b0ac31588ab90c"},{"author":{"_account_id":6524,"name":"Henry Gessau","email":"HenryG@gessau.net","username":"gessau"},"change_message_id":"59de2891000a63b2259f79a4565fd57cc4a6dc56","unresolved":false,"context_lines":[{"line_number":27,"context_line":""},{"line_number":28,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"# How much overhead does each of the tunnel types force?"},{"line_number":31,"context_line":"TUNNEL_OVERHEAD \u003d {"},{"line_number":32,"context_line":"    \u0027gre\u0027: 24,"},{"line_number":33,"context_line":"    \u0027vxlan\u0027: 50,"}],"source_content_type":"text/x-python","patch_set":1,"id":"9aa7fdbe_1f75e197","line":30,"in_reply_to":"9aa7fdbe_95770902","updated":"2014-10-22 03:29:17.000000000","message":"\"Bytes per packet of overhead incurred by each tunnel type.\"","commit_id":"5a8e1519a6805a310d6f573ee1b0ac31588ab90c"},{"author":{"_account_id":6524,"name":"Henry Gessau","email":"HenryG@gessau.net","username":"gessau"},"change_message_id":"59de2891000a63b2259f79a4565fd57cc4a6dc56","unresolved":false,"context_lines":[{"line_number":31,"context_line":"TUNNEL_OVERHEAD \u003d {"},{"line_number":32,"context_line":"    \u0027gre\u0027: 24,"},{"line_number":33,"context_line":"    \u0027vxlan\u0027: 50,"},{"line_number":34,"context_line":"    \u0027stt\u0027: 18,"},{"line_number":35,"context_line":"}"},{"line_number":36,"context_line":""},{"line_number":37,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"9aa7fdbe_df90b997","line":34,"updated":"2014-10-22 03:29:17.000000000","message":"Where did you get the number for STT?","commit_id":"5a8e1519a6805a310d6f573ee1b0ac31588ab90c"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"1b69cf1d627169ec350e5dd9f242a3cb931483b0","unresolved":false,"context_lines":[{"line_number":31,"context_line":"TUNNEL_OVERHEAD \u003d {"},{"line_number":32,"context_line":"    \u0027gre\u0027: 24,"},{"line_number":33,"context_line":"    \u0027vxlan\u0027: 50,"},{"line_number":34,"context_line":"    \u0027stt\u0027: 18,"},{"line_number":35,"context_line":"}"},{"line_number":36,"context_line":""},{"line_number":37,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"7a8c0949_31b66dda","line":34,"in_reply_to":"9aa7fdbe_053d7dde","updated":"2014-11-03 11:56:45.000000000","message":"The MTU configured using \u0027ifconfig\u0027 or the likes specify the biggest size of data *ethernet* packets can transmit - this does not include the Ethernet (MAC) headers, but does include raw data and L3+ headers (IP, TCP/UDP...) [1] [2].\n\nThe operation system should know to decrease the size of packet\u0027s data length compensating for IP headers (for example, IP header \u003d 20 bytes and UDP \u003d 8 bytes, so the maximum data that can be sent is 1472. STT shouldn\u0027t behave any differently than VXLAN and GRE in this regard.\n\nHope this answers your questions.\n\n[1] http://packetlife.net/blog/2008/nov/5/mtu-manipulation/\n[2] http://stackoverflow.com/questions/3928766/how-big-is-an-ip-packet-frame-including-headers","commit_id":"5a8e1519a6805a310d6f573ee1b0ac31588ab90c"},{"author":{"_account_id":6524,"name":"Henry Gessau","email":"HenryG@gessau.net","username":"gessau"},"change_message_id":"3a6335016f3894364c8677926499dd2e32d2baf4","unresolved":false,"context_lines":[{"line_number":31,"context_line":"TUNNEL_OVERHEAD \u003d {"},{"line_number":32,"context_line":"    \u0027gre\u0027: 24,"},{"line_number":33,"context_line":"    \u0027vxlan\u0027: 50,"},{"line_number":34,"context_line":"    \u0027stt\u0027: 18,"},{"line_number":35,"context_line":"}"},{"line_number":36,"context_line":""},{"line_number":37,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"9aa7fdbe_053d7dde","line":34,"in_reply_to":"9aa7fdbe_37df1c3d","updated":"2014-10-22 16:07:34.000000000","message":"OK this matches with http://tools.ietf.org/html/draft-davie-stt-06 but that draft also states:\n\n   frames are then segmented (depending on the MTU of the underlying\n   physical network) and the resulting segments are encapsulated in a\n   standard TCP header, which in turn is encapsulated by an IP header\n   and finally a MAC header.\n\nI have zero experience with STT but it sounds to me like it includes a mechanism for dealing with the underlying MTU. So we might not want to include it here with GRE and VXLAN. On the other hand, if the goal is to check whether underlying MTU is big enough to avoid STT segmentation, then the total overhead looks to be much more than just the 18 bytes of the STT frame format. Looks like at least 54 bytes?","commit_id":"5a8e1519a6805a310d6f573ee1b0ac31588ab90c"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"a89d711d24d06cc3a4cc1f2ab86e9073fd2f7381","unresolved":false,"context_lines":[{"line_number":31,"context_line":"TUNNEL_OVERHEAD \u003d {"},{"line_number":32,"context_line":"    \u0027gre\u0027: 24,"},{"line_number":33,"context_line":"    \u0027vxlan\u0027: 50,"},{"line_number":34,"context_line":"    \u0027stt\u0027: 18,"},{"line_number":35,"context_line":"}"},{"line_number":36,"context_line":""},{"line_number":37,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"9aa7fdbe_37df1c3d","line":34,"in_reply_to":"9aa7fdbe_df90b997","updated":"2014-10-22 12:40:34.000000000","message":"http://www.plexxi.com/2014/01/stateless-transport-tunneling-stt-meets-network/#sthash.EwRBR5bY.dpbs - diagram around midpage specifically states that STT\u0027s header is 18 bytes. Also saw some other reference to it someplace else so deemed it \"good enough for me\"","commit_id":"5a8e1519a6805a310d6f573ee1b0ac31588ab90c"},{"author":{"_account_id":12890,"name":"Vinod Pandarinathan","email":"vpandari@cisco.com","username":"Vinod"},"change_message_id":"96e88082dbe36cac1f9a7877749063ce374505db","unresolved":false,"context_lines":[{"line_number":116,"context_line":"        return False"},{"line_number":117,"context_line":""},{"line_number":118,"context_line":"    mtu \u003d int(match.group(\u0027mtu\u0027))"},{"line_number":119,"context_line":"    if mtu \u003c 1500 + _get_max_overhead(tunnel_types):"},{"line_number":120,"context_line":"        return False"},{"line_number":121,"context_line":"    return True"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a8c0949_4ef38873","line":119,"updated":"2014-10-30 09:53:13.000000000","message":"I have to agree with Henry, in most NFV use cases  jumbo frames are used, network_device_mtu from config files should be used if set.\n\nInaddition, it might be possible to check network_device_mtu and the underlying mtu of the interface ( bridge interface mtu might be less than network_device_mtu when the physical interface (attached to bridge) mtu is less than network_device_mtu  ). I do see network_device_mtu used in neutron.\n\nhttps://github.com/openstack/neutron/blob/master/neutron/agent/linux/interface.py","commit_id":"5a8e1519a6805a310d6f573ee1b0ac31588ab90c"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"c4a526db0ec31158b1d4c79f0e773801807aa962","unresolved":false,"context_lines":[{"line_number":116,"context_line":"        return False"},{"line_number":117,"context_line":""},{"line_number":118,"context_line":"    mtu \u003d int(match.group(\u0027mtu\u0027))"},{"line_number":119,"context_line":"    if mtu \u003c 1500 + _get_max_overhead(tunnel_types):"},{"line_number":120,"context_line":"        return False"},{"line_number":121,"context_line":"    return True"}],"source_content_type":"text/x-python","patch_set":1,"id":"9aa7fdbe_37a71c0e","line":119,"updated":"2014-10-22 12:23:53.000000000","message":"Is this always going to be 1500? If openvswitch has support for jumbo frames and the VMs are configured to use them, this could be much larger.","commit_id":"5a8e1519a6805a310d6f573ee1b0ac31588ab90c"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"82a828ea1de6ca5b36000c4efb0758be5b6c1818","unresolved":false,"context_lines":[{"line_number":116,"context_line":"        return False"},{"line_number":117,"context_line":""},{"line_number":118,"context_line":"    mtu \u003d int(match.group(\u0027mtu\u0027))"},{"line_number":119,"context_line":"    if mtu \u003c 1500 + _get_max_overhead(tunnel_types):"},{"line_number":120,"context_line":"        return False"},{"line_number":121,"context_line":"    return True"}],"source_content_type":"text/x-python","patch_set":1,"id":"9aa7fdbe_f5ccd537","line":119,"updated":"2014-10-21 12:10:30.000000000","message":"i do not get the 1500. what of there are jumbo frames enabled on the network","commit_id":"5a8e1519a6805a310d6f573ee1b0ac31588ab90c"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"1b69cf1d627169ec350e5dd9f242a3cb931483b0","unresolved":false,"context_lines":[{"line_number":116,"context_line":"        return False"},{"line_number":117,"context_line":""},{"line_number":118,"context_line":"    mtu \u003d int(match.group(\u0027mtu\u0027))"},{"line_number":119,"context_line":"    if mtu \u003c 1500 + _get_max_overhead(tunnel_types):"},{"line_number":120,"context_line":"        return False"},{"line_number":121,"context_line":"    return True"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a8c0949_f17ea5e3","line":119,"in_reply_to":"7a8c0949_4ef38873","updated":"2014-11-03 11:56:45.000000000","message":"It is my understanding the neutron\u0027s network_device_mtu is used for interfaces that are created by the L3 agent, DHCP agent and such - tap devices, etc. It would be wrong IMO to use this option.\n\nThe option we *do* want to use is nova\u0027s network_device_mtu, which is eventually used to create the VM to br-int. However I\u0027m not sure how good of an idea it is to lookup nova\u0027s configuration in this script, which we don\u0027t even know where the configuration file was deployed.\n\n@Henry, I could add a \u0027-nova-mtu\u0027 but that seems a little vague and requires more explanations to the user running the script, ending up confusing.\n\nI\u0027m fine with leaving this as is until such a time when https://review.openstack.org/#/c/105989/ is merged. Any opinions on this matter?","commit_id":"5a8e1519a6805a310d6f573ee1b0ac31588ab90c"},{"author":{"_account_id":10061,"name":"Joe Talerico","email":"jtaleric@redhat.com","username":"rook"},"change_message_id":"c758d0cb6bacdac6b017a4f61255ff556edd3433","unresolved":false,"context_lines":[{"line_number":116,"context_line":"        return False"},{"line_number":117,"context_line":""},{"line_number":118,"context_line":"    mtu \u003d int(match.group(\u0027mtu\u0027))"},{"line_number":119,"context_line":"    if mtu \u003c 1500 + _get_max_overhead(tunnel_types):"},{"line_number":120,"context_line":"        return False"},{"line_number":121,"context_line":"    return True"}],"source_content_type":"text/x-python","patch_set":1,"id":"9aa7fdbe_77736480","line":119,"in_reply_to":"9aa7fdbe_37a71c0e","updated":"2014-10-22 12:26:39.000000000","message":"I think this was just a assumption. So, there are a few options, query the dhcp options to determine how dhcp was configured with option 26, or look at the guest tap devices to determine if network_device_mtu is set... just spit balling ideas!","commit_id":"5a8e1519a6805a310d6f573ee1b0ac31588ab90c"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"a89d711d24d06cc3a4cc1f2ab86e9073fd2f7381","unresolved":false,"context_lines":[{"line_number":116,"context_line":"        return False"},{"line_number":117,"context_line":""},{"line_number":118,"context_line":"    mtu \u003d int(match.group(\u0027mtu\u0027))"},{"line_number":119,"context_line":"    if mtu \u003c 1500 + _get_max_overhead(tunnel_types):"},{"line_number":120,"context_line":"        return False"},{"line_number":121,"context_line":"    return True"}],"source_content_type":"text/x-python","patch_set":1,"id":"9aa7fdbe_b74f8cbc","line":119,"in_reply_to":"9aa7fdbe_77736480","updated":"2014-10-22 12:40:34.000000000","message":"I used the assumption that most installations now-a-days start as a default of 1500 MTU. This includes most LiveCDs (IMO, do correct me if I\u0027m wrong), so used that as a default.\n\n@Gary, jumbo frames could also not be enabled. Supposed there exists a setup with physical routers which don\u0027t support jumbo frames at all - all packets which are bigger than 1500 is deemed unworthy on receipt and thrown out the door.\n\n@Henry, there are currently numerous ways to setup a desired MTU for the VMs (naming a few - using the dhcp agent to force MTU on instances, users installing images with higher-than-1500 MTU, and admins running manual tools to provision new instances which change the MTU). There simply isn\u0027t \"one\" way to specify VMs\u0027 MTU, so how can we know which one to check in this sanity check?\n\n@Kevin, the VMs may support jumbo frames, but what if the physical infra doesn\u0027t?\n\n@Joe, checking nova\u0027s network_device_mtu (I assume you talked about that since that\u0027s the one which matters here) is a bit \"out of reach\" for a Neutron-sanity-checker-script (since that\u0027s a nova configuration). What do you think?\n\nI think this one is for the ML. What do you guys reckon?","commit_id":"5a8e1519a6805a310d6f573ee1b0ac31588ab90c"},{"author":{"_account_id":6524,"name":"Henry Gessau","email":"HenryG@gessau.net","username":"gessau"},"change_message_id":"3a6335016f3894364c8677926499dd2e32d2baf4","unresolved":false,"context_lines":[{"line_number":116,"context_line":"        return False"},{"line_number":117,"context_line":""},{"line_number":118,"context_line":"    mtu \u003d int(match.group(\u0027mtu\u0027))"},{"line_number":119,"context_line":"    if mtu \u003c 1500 + _get_max_overhead(tunnel_types):"},{"line_number":120,"context_line":"        return False"},{"line_number":121,"context_line":"    return True"}],"source_content_type":"text/x-python","patch_set":1,"id":"9aa7fdbe_0052cb12","line":119,"in_reply_to":"9aa7fdbe_b74f8cbc","updated":"2014-10-22 16:07:34.000000000","message":"Sorry for not being clear. I meant that the operator running the check could specify the MTU desired for guest VMs, for example:\n\n   neutron-sanity-check --check_mtu guest-mtu\u003d2000\n\n(There may be a better way to specify the option.)","commit_id":"5a8e1519a6805a310d6f573ee1b0ac31588ab90c"},{"author":{"_account_id":6524,"name":"Henry Gessau","email":"HenryG@gessau.net","username":"gessau"},"change_message_id":"59de2891000a63b2259f79a4565fd57cc4a6dc56","unresolved":false,"context_lines":[{"line_number":116,"context_line":"        return False"},{"line_number":117,"context_line":""},{"line_number":118,"context_line":"    mtu \u003d int(match.group(\u0027mtu\u0027))"},{"line_number":119,"context_line":"    if mtu \u003c 1500 + _get_max_overhead(tunnel_types):"},{"line_number":120,"context_line":"        return False"},{"line_number":121,"context_line":"    return True"}],"source_content_type":"text/x-python","patch_set":1,"id":"9aa7fdbe_6242d611","line":119,"in_reply_to":"9aa7fdbe_f5ccd537","updated":"2014-10-22 03:29:17.000000000","message":"Here \u0027mtu\u0027 is the value for the host interface for the local endpoint of the tunnels. The default for VM interfaces is 1500. If the host interface is enabled for jumbo frames then mtu will be some large number like 9000.\n\nI think we can do better than hard-coding 1500. How about default to 1500 but have the option to specify a desired MTU for the VMs?","commit_id":"5a8e1519a6805a310d6f573ee1b0ac31588ab90c"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"055fddfe35ad998250d6cc53564f6f99f77282b9","unresolved":false,"context_lines":[{"line_number":125,"context_line":"def is_mtu_valid(root_helper, local_ip, tunnel_types):"},{"line_number":126,"context_line":"    args \u003d [\u0027ip\u0027, \u0027addr\u0027, \u0027show\u0027, \u0027to\u0027, local_ip]"},{"line_number":127,"context_line":"    output \u003d agent_utils.execute(args, root_helper\u003droot_helper)"},{"line_number":128,"context_line":"    match \u003d re.search(\u0027mtu (?P\u003cmtu\u003e\\d*)\u0027, output)"},{"line_number":129,"context_line":""},{"line_number":130,"context_line":"    if not match:"},{"line_number":131,"context_line":"        LOG.warning(_LW(\u0027Could not discover mtu of device owning %s.\u0027),"}],"source_content_type":"text/x-python","patch_set":2,"id":"5a890539_b217bfec","line":128,"updated":"2014-11-11 23:24:57.000000000","message":"Nit: Should that be \u0027mtu (?P\u003cmtu\u003e\\d+)\u0027?","commit_id":"7ab07e80dad964281a59c8e2465f6214c8483bbe"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"8a4ef221a94dc7eb8d59b05bc2604f7504516ec5","unresolved":false,"context_lines":[{"line_number":125,"context_line":"def is_mtu_valid(root_helper, local_ip, tunnel_types):"},{"line_number":126,"context_line":"    args \u003d [\u0027ip\u0027, \u0027addr\u0027, \u0027show\u0027, \u0027to\u0027, local_ip]"},{"line_number":127,"context_line":"    output \u003d agent_utils.execute(args, root_helper\u003droot_helper)"},{"line_number":128,"context_line":"    match \u003d re.search(\u0027mtu (?P\u003cmtu\u003e\\d*)\u0027, output)"},{"line_number":129,"context_line":""},{"line_number":130,"context_line":"    if not match:"},{"line_number":131,"context_line":"        LOG.warning(_LW(\u0027Could not discover mtu of device owning %s.\u0027),"}],"source_content_type":"text/x-python","patch_set":2,"id":"5a890539_9130f79f","line":128,"in_reply_to":"5a890539_b217bfec","updated":"2014-11-17 07:15:43.000000000","message":"It should. Nice pickup :)","commit_id":"7ab07e80dad964281a59c8e2465f6214c8483bbe"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"055fddfe35ad998250d6cc53564f6f99f77282b9","unresolved":false,"context_lines":[{"line_number":133,"context_line":"        return False"},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"    current_mtu \u003d int(match.group(\u0027mtu\u0027))"},{"line_number":136,"context_line":"    needed_mtu \u003d 1500 + _get_max_overhead(tunnel_types)"},{"line_number":137,"context_line":""},{"line_number":138,"context_line":"    if current_mtu \u003c needed_mtu:"},{"line_number":139,"context_line":"        LOG.warning(_LW(\u0027The MTU of the physical interface hodling IP \u0027"}],"source_content_type":"text/x-python","patch_set":2,"id":"5a890539_32baefde","line":136,"updated":"2014-11-11 23:24:57.000000000","message":"Nit: Should there be a constant created for default MTU, vs hard-coded value?","commit_id":"7ab07e80dad964281a59c8e2465f6214c8483bbe"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"8a4ef221a94dc7eb8d59b05bc2604f7504516ec5","unresolved":false,"context_lines":[{"line_number":133,"context_line":"        return False"},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"    current_mtu \u003d int(match.group(\u0027mtu\u0027))"},{"line_number":136,"context_line":"    needed_mtu \u003d 1500 + _get_max_overhead(tunnel_types)"},{"line_number":137,"context_line":""},{"line_number":138,"context_line":"    if current_mtu \u003c needed_mtu:"},{"line_number":139,"context_line":"        LOG.warning(_LW(\u0027The MTU of the physical interface hodling IP \u0027"}],"source_content_type":"text/x-python","patch_set":2,"id":"5a890539_b1ded33d","line":136,"in_reply_to":"5a890539_32baefde","updated":"2014-11-17 07:15:43.000000000","message":"Done","commit_id":"7ab07e80dad964281a59c8e2465f6214c8483bbe"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"055fddfe35ad998250d6cc53564f6f99f77282b9","unresolved":false,"context_lines":[{"line_number":136,"context_line":"    needed_mtu \u003d 1500 + _get_max_overhead(tunnel_types)"},{"line_number":137,"context_line":""},{"line_number":138,"context_line":"    if current_mtu \u003c needed_mtu:"},{"line_number":139,"context_line":"        LOG.warning(_LW(\u0027The MTU of the physical interface hodling IP \u0027"},{"line_number":140,"context_line":"                        \u0027%(local_ip)s is %(current_mtu)s, but should be \u0027"},{"line_number":141,"context_line":"                        \u0027raised to %(needed_mtu)s!\u0027),"},{"line_number":142,"context_line":"                    {\u0027local_ip\u0027: local_ip,"}],"source_content_type":"text/x-python","patch_set":2,"id":"5a890539_f28dc732","line":139,"updated":"2014-11-11 23:24:57.000000000","message":"hodling -\u003e holding","commit_id":"7ab07e80dad964281a59c8e2465f6214c8483bbe"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"8a4ef221a94dc7eb8d59b05bc2604f7504516ec5","unresolved":false,"context_lines":[{"line_number":136,"context_line":"    needed_mtu \u003d 1500 + _get_max_overhead(tunnel_types)"},{"line_number":137,"context_line":""},{"line_number":138,"context_line":"    if current_mtu \u003c needed_mtu:"},{"line_number":139,"context_line":"        LOG.warning(_LW(\u0027The MTU of the physical interface hodling IP \u0027"},{"line_number":140,"context_line":"                        \u0027%(local_ip)s is %(current_mtu)s, but should be \u0027"},{"line_number":141,"context_line":"                        \u0027raised to %(needed_mtu)s!\u0027),"},{"line_number":142,"context_line":"                    {\u0027local_ip\u0027: local_ip,"}],"source_content_type":"text/x-python","patch_set":2,"id":"5a890539_d1e19fff","line":139,"in_reply_to":"5a890539_f28dc732","updated":"2014-11-17 07:15:43.000000000","message":"Done","commit_id":"7ab07e80dad964281a59c8e2465f6214c8483bbe"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"055fddfe35ad998250d6cc53564f6f99f77282b9","unresolved":false,"context_lines":[{"line_number":138,"context_line":"    if current_mtu \u003c needed_mtu:"},{"line_number":139,"context_line":"        LOG.warning(_LW(\u0027The MTU of the physical interface hodling IP \u0027"},{"line_number":140,"context_line":"                        \u0027%(local_ip)s is %(current_mtu)s, but should be \u0027"},{"line_number":141,"context_line":"                        \u0027raised to %(needed_mtu)s!\u0027),"},{"line_number":142,"context_line":"                    {\u0027local_ip\u0027: local_ip,"},{"line_number":143,"context_line":"                     \u0027current_mtu\u0027: current_mtu,"},{"line_number":144,"context_line":"                     \u0027needed_mtu\u0027: needed_mtu})"}],"source_content_type":"text/x-python","patch_set":2,"id":"5a890539_12833305","line":141,"updated":"2014-11-11 23:24:57.000000000","message":"raised to -\u003e at least","commit_id":"7ab07e80dad964281a59c8e2465f6214c8483bbe"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"8a4ef221a94dc7eb8d59b05bc2604f7504516ec5","unresolved":false,"context_lines":[{"line_number":138,"context_line":"    if current_mtu \u003c needed_mtu:"},{"line_number":139,"context_line":"        LOG.warning(_LW(\u0027The MTU of the physical interface hodling IP \u0027"},{"line_number":140,"context_line":"                        \u0027%(local_ip)s is %(current_mtu)s, but should be \u0027"},{"line_number":141,"context_line":"                        \u0027raised to %(needed_mtu)s!\u0027),"},{"line_number":142,"context_line":"                    {\u0027local_ip\u0027: local_ip,"},{"line_number":143,"context_line":"                     \u0027current_mtu\u0027: current_mtu,"},{"line_number":144,"context_line":"                     \u0027needed_mtu\u0027: needed_mtu})"}],"source_content_type":"text/x-python","patch_set":2,"id":"5a890539_f1e77b10","line":141,"in_reply_to":"5a890539_12833305","updated":"2014-11-17 07:15:43.000000000","message":"Done","commit_id":"7ab07e80dad964281a59c8e2465f6214c8483bbe"},{"author":{"_account_id":6524,"name":"Henry Gessau","email":"HenryG@gessau.net","username":"gessau"},"change_message_id":"15e2474e236f728118bfa97571debd4de3707875","unresolved":false,"context_lines":[{"line_number":32,"context_line":"TUNNEL_OVERHEAD \u003d {"},{"line_number":33,"context_line":"    \u0027gre\u0027: 24,"},{"line_number":34,"context_line":"    \u0027vxlan\u0027: 50,"},{"line_number":35,"context_line":"    \u0027stt\u0027: 18,"},{"line_number":36,"context_line":"}"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"# The MTU most machines start with. Does not indicate the effective default MTU"}],"source_content_type":"text/x-python","patch_set":5,"id":"5a890539_82956734","line":35,"updated":"2014-11-18 05:05:08.000000000","message":"Sorry for the late reply. This is a follow-up to the discussion about STT in patch set 1.\n\nAfter reading [1] I think that lowering the MTU will not help STT. In fact it will make things worse, I think. I may of course be completely wrong. Perhaps we could try to get a more authoritative reply from someone at VMware who knows STT?\n\n[1] http://www.plexxi.com/2014/01/stateless-transport-tunneling-stt-meets-network","commit_id":"8e4297b3c96b071a3e251833fb5125fd957d4f7e"},{"author":{"_account_id":261,"name":"Salvatore Orlando","email":"salv.orlando@gmail.com","username":"salvatore-orlando"},"change_message_id":"ceaf478213d4d94e50c594f9941616266d8ef2ee","unresolved":false,"context_lines":[{"line_number":32,"context_line":"TUNNEL_OVERHEAD \u003d {"},{"line_number":33,"context_line":"    \u0027gre\u0027: 24,"},{"line_number":34,"context_line":"    \u0027vxlan\u0027: 50,"},{"line_number":35,"context_line":"    \u0027stt\u0027: 18,"},{"line_number":36,"context_line":"}"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"# The MTU most machines start with. Does not indicate the effective default MTU"}],"source_content_type":"text/x-python","patch_set":5,"id":"5a890539_b042f20c","line":35,"in_reply_to":"5a890539_82956734","updated":"2014-12-03 09:53:01.000000000","message":"I don\u0027t think we need to lower the MTU for STT. Anyway, I would not be surprised if at the end of they day I end up being totally wrong.\n\n\nSo bear with me for a bit while I come back to you with an appropriate answer.","commit_id":"8e4297b3c96b071a3e251833fb5125fd957d4f7e"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"7c85eca6ae4bdd495e6212b1691cff64c7630b31","unresolved":false,"context_lines":[{"line_number":32,"context_line":"TUNNEL_OVERHEAD \u003d {"},{"line_number":33,"context_line":"    \u0027gre\u0027: 24,"},{"line_number":34,"context_line":"    \u0027vxlan\u0027: 50,"},{"line_number":35,"context_line":"    \u0027stt\u0027: 18,"},{"line_number":36,"context_line":"}"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"# The MTU most machines start with. Does not indicate the effective default MTU"}],"source_content_type":"text/x-python","patch_set":5,"id":"3a961159_215371e3","line":35,"in_reply_to":"5a890539_b042f20c","updated":"2015-01-13 07:59:26.000000000","message":"@Henry, looking at [1], STT is supposed to address MTU the same way VXLAN and GRE does. I re-read the link you posted and I don\u0027t see why lowering the MTU won\u0027t help STT.\n\nThe way I see it, STT still fragments the packets into smaller packets so they can fit the MTU limitation, so if the client\u0027s MTU isn\u0027t low enough to make sure STT\u0027s header gets in each packet, the packet will be dropped somewhere in Neutron\u0027s interfaces.\n\nEither way, it seems that STT adds 54 bytes [2] [3]...\n\n(sorry for the late response in this matter...)\n\n[1]: https://tools.ietf.org/html/draft-davie-stt-04\n[2]: http://www.networkworld.com/article/2224654/cisco-subnet/mtu-size-issues.html\n[3]: http://baturin.org/tools/encapcalc/","commit_id":"8e4297b3c96b071a3e251833fb5125fd957d4f7e"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"ac64ba455183e0765f5a03a5cabd27ab159aab21","unresolved":false,"context_lines":[{"line_number":123,"context_line":""},{"line_number":124,"context_line":""},{"line_number":125,"context_line":"def _get_max_overhead(tunnel_types):"},{"line_number":126,"context_line":"    return max([TUNNEL_OVERHEAD[tunnel] for tunnel in tunnel_types])"},{"line_number":127,"context_line":""},{"line_number":128,"context_line":""},{"line_number":129,"context_line":"def is_mtu_valid(local_ip, tunnel_types):"}],"source_content_type":"text/x-python","patch_set":5,"id":"3a961159_67014904","line":126,"updated":"2015-01-09 20:50:22.000000000","message":"the list is not required:\n\n  return max(TUNNEL_OVERHEAD[tunnel] for tunnel in tunnel_types)","commit_id":"8e4297b3c96b071a3e251833fb5125fd957d4f7e"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"7c85eca6ae4bdd495e6212b1691cff64c7630b31","unresolved":false,"context_lines":[{"line_number":123,"context_line":""},{"line_number":124,"context_line":""},{"line_number":125,"context_line":"def _get_max_overhead(tunnel_types):"},{"line_number":126,"context_line":"    return max([TUNNEL_OVERHEAD[tunnel] for tunnel in tunnel_types])"},{"line_number":127,"context_line":""},{"line_number":128,"context_line":""},{"line_number":129,"context_line":"def is_mtu_valid(local_ip, tunnel_types):"}],"source_content_type":"text/x-python","patch_set":5,"id":"3a961159_2101b1af","line":126,"in_reply_to":"3a961159_67014904","updated":"2015-01-13 07:59:26.000000000","message":"Done","commit_id":"8e4297b3c96b071a3e251833fb5125fd957d4f7e"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"bdadc1afcc8cee10866ca252745ae4efc0db5527","unresolved":false,"context_lines":[{"line_number":145,"context_line":"    return not exists"},{"line_number":146,"context_line":""},{"line_number":147,"context_line":""},{"line_number":148,"context_line":"def _get_max_overhead(tunnel_types):"},{"line_number":149,"context_line":"    return max(TUNNEL_OVERHEAD[tunnel] for tunnel in tunnel_types)"},{"line_number":150,"context_line":""},{"line_number":151,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"3a961159_437061a4","line":148,"updated":"2015-01-14 00:36:25.000000000","message":"Does this justify a method?","commit_id":"a0c572b7a57638424caae7a95d2a6e95231f60ae"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"f03673f0c4fbdaba5ae24e0a1bbd821e97b364f9","unresolved":false,"context_lines":[{"line_number":145,"context_line":"    return not exists"},{"line_number":146,"context_line":""},{"line_number":147,"context_line":""},{"line_number":148,"context_line":"def _get_max_overhead(tunnel_types):"},{"line_number":149,"context_line":"    return max(TUNNEL_OVERHEAD[tunnel] for tunnel in tunnel_types)"},{"line_number":150,"context_line":""},{"line_number":151,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"3a961159_64e399a4","line":148,"in_reply_to":"3a961159_437061a4","updated":"2015-01-14 10:31:33.000000000","message":"Now it doesn\u0027t :)","commit_id":"a0c572b7a57638424caae7a95d2a6e95231f60ae"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"bdadc1afcc8cee10866ca252745ae4efc0db5527","unresolved":false,"context_lines":[{"line_number":166,"context_line":"    if not current_mtu:"},{"line_number":167,"context_line":"        return False"},{"line_number":168,"context_line":""},{"line_number":169,"context_line":"    needed_mtu \u003d DEFAULT_MTU + _get_max_overhead(tunnel_types)"},{"line_number":170,"context_line":""},{"line_number":171,"context_line":"    if current_mtu \u003c needed_mtu:"},{"line_number":172,"context_line":"        LOG.warning(_LW(\u0027The MTU of the physical interface holding IP \u0027"}],"source_content_type":"text/x-python","patch_set":7,"id":"3a961159_e3e78d44","line":169,"updated":"2015-01-14 00:36:25.000000000","message":"Sometimes, I think we go overboard with indirection.\n\n max_overhead \u003d max(TUNNEL_OVERHEAD[tunnel] for tunnel in tunnel_types)\n needed_mtu \u003d DEFAULT_MTU + max_overhead","commit_id":"a0c572b7a57638424caae7a95d2a6e95231f60ae"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"f03673f0c4fbdaba5ae24e0a1bbd821e97b364f9","unresolved":false,"context_lines":[{"line_number":166,"context_line":"    if not current_mtu:"},{"line_number":167,"context_line":"        return False"},{"line_number":168,"context_line":""},{"line_number":169,"context_line":"    needed_mtu \u003d DEFAULT_MTU + _get_max_overhead(tunnel_types)"},{"line_number":170,"context_line":""},{"line_number":171,"context_line":"    if current_mtu \u003c needed_mtu:"},{"line_number":172,"context_line":"        LOG.warning(_LW(\u0027The MTU of the physical interface holding IP \u0027"}],"source_content_type":"text/x-python","patch_set":7,"id":"3a961159_e4e6a9af","line":169,"in_reply_to":"3a961159_09d3d3d5","updated":"2015-01-14 10:31:33.000000000","message":"Done","commit_id":"a0c572b7a57638424caae7a95d2a6e95231f60ae"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"e38876b52a2d6d571a57377932c9d1cd8c8a8d05","unresolved":false,"context_lines":[{"line_number":166,"context_line":"    if not current_mtu:"},{"line_number":167,"context_line":"        return False"},{"line_number":168,"context_line":""},{"line_number":169,"context_line":"    needed_mtu \u003d DEFAULT_MTU + _get_max_overhead(tunnel_types)"},{"line_number":170,"context_line":""},{"line_number":171,"context_line":"    if current_mtu \u003c needed_mtu:"},{"line_number":172,"context_line":"        LOG.warning(_LW(\u0027The MTU of the physical interface holding IP \u0027"}],"source_content_type":"text/x-python","patch_set":7,"id":"3a961159_09d3d3d5","line":169,"in_reply_to":"3a961159_e3e78d44","updated":"2015-01-14 07:56:13.000000000","message":"I believe both approaches are ok :-), we shouldn\u0027t need to go too much picky about \"excessive\" indirection, because the other way (non enough) is much worse IMHO.\n\nBtw, what Carl proposes is in a very good balance and \"max(TUNNEL_OVERHEAD[....   \" is very idiomatic by it\u0027s own.","commit_id":"a0c572b7a57638424caae7a95d2a6e95231f60ae"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"bdadc1afcc8cee10866ca252745ae4efc0db5527","unresolved":false,"context_lines":[{"line_number":171,"context_line":"    if current_mtu \u003c needed_mtu:"},{"line_number":172,"context_line":"        LOG.warning(_LW(\u0027The MTU of the physical interface holding IP \u0027"},{"line_number":173,"context_line":"                        \u0027%(local_ip)s is %(current_mtu)s, but should be \u0027"},{"line_number":174,"context_line":"                        \u0027at least %(needed_mtu)s!\u0027),"},{"line_number":175,"context_line":"                    {\u0027local_ip\u0027: local_ip,"},{"line_number":176,"context_line":"                     \u0027current_mtu\u0027: current_mtu,"},{"line_number":177,"context_line":"                     \u0027needed_mtu\u0027: needed_mtu})"}],"source_content_type":"text/x-python","patch_set":7,"id":"3a961159_e3b86dea","line":174,"updated":"2015-01-14 00:36:25.000000000","message":"You might consider printing a bit more information about the overhead here so that the operator will know how needed_mtu was calculated.  Then, maybe append \" for the configured tunnel types\" at the end.","commit_id":"a0c572b7a57638424caae7a95d2a6e95231f60ae"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"f03673f0c4fbdaba5ae24e0a1bbd821e97b364f9","unresolved":false,"context_lines":[{"line_number":171,"context_line":"    if current_mtu \u003c needed_mtu:"},{"line_number":172,"context_line":"        LOG.warning(_LW(\u0027The MTU of the physical interface holding IP \u0027"},{"line_number":173,"context_line":"                        \u0027%(local_ip)s is %(current_mtu)s, but should be \u0027"},{"line_number":174,"context_line":"                        \u0027at least %(needed_mtu)s!\u0027),"},{"line_number":175,"context_line":"                    {\u0027local_ip\u0027: local_ip,"},{"line_number":176,"context_line":"                     \u0027current_mtu\u0027: current_mtu,"},{"line_number":177,"context_line":"                     \u0027needed_mtu\u0027: needed_mtu})"}],"source_content_type":"text/x-python","patch_set":7,"id":"3a961159_44fbfd47","line":174,"in_reply_to":"3a961159_095cb37c","updated":"2015-01-14 10:31:33.000000000","message":"Done. Is this better?","commit_id":"a0c572b7a57638424caae7a95d2a6e95231f60ae"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"e38876b52a2d6d571a57377932c9d1cd8c8a8d05","unresolved":false,"context_lines":[{"line_number":171,"context_line":"    if current_mtu \u003c needed_mtu:"},{"line_number":172,"context_line":"        LOG.warning(_LW(\u0027The MTU of the physical interface holding IP \u0027"},{"line_number":173,"context_line":"                        \u0027%(local_ip)s is %(current_mtu)s, but should be \u0027"},{"line_number":174,"context_line":"                        \u0027at least %(needed_mtu)s!\u0027),"},{"line_number":175,"context_line":"                    {\u0027local_ip\u0027: local_ip,"},{"line_number":176,"context_line":"                     \u0027current_mtu\u0027: current_mtu,"},{"line_number":177,"context_line":"                     \u0027needed_mtu\u0027: needed_mtu})"}],"source_content_type":"text/x-python","patch_set":7,"id":"3a961159_095cb37c","line":174,"in_reply_to":"3a961159_e3b86dea","updated":"2015-01-14 07:56:13.000000000","message":"Yes, may be a per-tunnel-type recommended configuration would be interesting.","commit_id":"a0c572b7a57638424caae7a95d2a6e95231f60ae"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"bbff3817a85e7f2211bd9013635f698f34938079","unresolved":false,"context_lines":[{"line_number":176,"context_line":"    \"\"\""},{"line_number":177,"context_line":"    local_ip \u003d ovs_local_ip or vxlan_local_ip"},{"line_number":178,"context_line":""},{"line_number":179,"context_line":"    if not local_ip:"},{"line_number":180,"context_line":"        LOG.error(_LE(\"The local_ip option must be set in either \""},{"line_number":181,"context_line":"                      \"openvswitch\u0027s configuration (under the [ovs] section) \""},{"line_number":182,"context_line":"                      \"and in linuxbridge\u0027s configuration (under the [vxlan] \""}],"source_content_type":"text/x-python","patch_set":9,"id":"3a961159_25bd6f04","line":179,"updated":"2015-01-19 22:27:45.000000000","message":"it seems L179  and L148 tests are redundant","commit_id":"862edba89d598d97ef3ba6d2aa66d1d372c2de8f"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"dd5532bde674c6db1877d1a179e9bd3504d326aa","unresolved":false,"context_lines":[{"line_number":176,"context_line":"    \"\"\""},{"line_number":177,"context_line":"    local_ip \u003d ovs_local_ip or vxlan_local_ip"},{"line_number":178,"context_line":""},{"line_number":179,"context_line":"    if not local_ip:"},{"line_number":180,"context_line":"        LOG.error(_LE(\"The local_ip option must be set in either \""},{"line_number":181,"context_line":"                      \"openvswitch\u0027s configuration (under the [ovs] section) \""},{"line_number":182,"context_line":"                      \"and in linuxbridge\u0027s configuration (under the [vxlan] \""}],"source_content_type":"text/x-python","patch_set":9,"id":"3a961159_f30b26d3","line":179,"in_reply_to":"3a961159_25bd6f04","updated":"2015-01-20 07:49:32.000000000","message":"They actually aren\u0027t. For example, one could define both VXLAN and OVS options for local_ip to 2 different IPs, and then a conflict will occur (and _is_local_ip_conflicting should catch it).\n\nHowever, if only VXLAN\u0027s local_ip option is specified (and OVS\u0027s defaults to None), then I\u0027m not going to force them to match one another (each one is used by a different plugin) so there isn\u0027t a conflict.","commit_id":"862edba89d598d97ef3ba6d2aa66d1d372c2de8f"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"46e0fe8b802013488337da351396b1b4ea338390","unresolved":false,"context_lines":[{"line_number":172,"context_line":"    return False"},{"line_number":173,"context_line":""},{"line_number":174,"context_line":""},{"line_number":175,"context_line":"def get_local_ip(ovs_local_ip, vxlan_local_ip):"},{"line_number":176,"context_line":"    \"\"\"Retrieve the \u0027local_ip\u0027 option from the config and make sure it\u0027s valid."},{"line_number":177,"context_line":""},{"line_number":178,"context_line":"    \u0027local_ip\u0027 is configured in the OVS (for openvswitch) and in the VXLAN (for"}],"source_content_type":"text/x-python","patch_set":10,"id":"1a930d6b_3465b645","line":175,"updated":"2015-01-20 21:50:50.000000000","message":"Is there any reason for this and _is_local_ip_valid to a) take ovs_local_ip and vxlan_local_ip instead of just grabbing the correct one from the config in the first place and just passing it and b) to be separate functions? get_local_ip does absolutely nothing by itself other than check args.","commit_id":"537d72c3d20571f0552927675b5c8cc8604e3d19"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"669ebfdd3bd7bbdf2daec7ce45e763903689c0b6","unresolved":false,"context_lines":[{"line_number":172,"context_line":"    return False"},{"line_number":173,"context_line":""},{"line_number":174,"context_line":""},{"line_number":175,"context_line":"def get_local_ip(ovs_local_ip, vxlan_local_ip):"},{"line_number":176,"context_line":"    \"\"\"Retrieve the \u0027local_ip\u0027 option from the config and make sure it\u0027s valid."},{"line_number":177,"context_line":""},{"line_number":178,"context_line":"    \u0027local_ip\u0027 is configured in the OVS (for openvswitch) and in the VXLAN (for"}],"source_content_type":"text/x-python","patch_set":10,"id":"1a930d6b_bd5fbd71","line":175,"in_reply_to":"1a930d6b_3465b645","updated":"2015-01-21 09:39:13.000000000","message":"a) The current code focuses all oslo config access to sanity_checks.py (the other files), and have this file only check the validity of the values. Since you mentioned it I\u0027ll fix the code.\n\nb) The separation is to allow proper unit test checks that don\u0027t rely on calls to LOG.xxx. See earlier patchsets.","commit_id":"537d72c3d20571f0552927675b5c8cc8604e3d19"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"5b4d1d9431f1e70d68a0a254c91baf8c09dd3a4c","unresolved":false,"context_lines":[{"line_number":187,"context_line":"                      \"and in linuxbridge\u0027s configuration (under the [vxlan] \""},{"line_number":188,"context_line":"                      \"section). if they are both present, they must match.\"))"},{"line_number":189,"context_line":"        return None"},{"line_number":190,"context_line":"    elif not _is_local_ip_valid(ovs_local_ip, vxlan_local_ip):"},{"line_number":191,"context_line":"        return None"},{"line_number":192,"context_line":""},{"line_number":193,"context_line":"    return local_ip"}],"source_content_type":"text/x-python","patch_set":10,"id":"3a961159_58787c2c","line":190,"updated":"2015-01-20 19:44:02.000000000","message":"nit: can just be:\n    if not _is_local_ip_valid(...):","commit_id":"537d72c3d20571f0552927675b5c8cc8604e3d19"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"d1218220270b961df170d215d0566216f556ac82","unresolved":false,"context_lines":[{"line_number":187,"context_line":"                      \"and in linuxbridge\u0027s configuration (under the [vxlan] \""},{"line_number":188,"context_line":"                      \"section). if they are both present, they must match.\"))"},{"line_number":189,"context_line":"        return None"},{"line_number":190,"context_line":"    elif not _is_local_ip_valid(ovs_local_ip, vxlan_local_ip):"},{"line_number":191,"context_line":"        return None"},{"line_number":192,"context_line":""},{"line_number":193,"context_line":"    return local_ip"}],"source_content_type":"text/x-python","patch_set":10,"id":"1a930d6b_b0b437f3","line":190,"in_reply_to":"1a930d6b_5dc7b912","updated":"2015-01-21 15:13:42.000000000","message":"That\u0027s my point. The only way this statement can be evaluated is if the condition on L184 is false. Therefore the elif is extraneous, and can just be an if.","commit_id":"537d72c3d20571f0552927675b5c8cc8604e3d19"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"fe18eea60b10a8e1e462886485874dd71445fcc3","unresolved":false,"context_lines":[{"line_number":187,"context_line":"                      \"and in linuxbridge\u0027s configuration (under the [vxlan] \""},{"line_number":188,"context_line":"                      \"section). if they are both present, they must match.\"))"},{"line_number":189,"context_line":"        return None"},{"line_number":190,"context_line":"    elif not _is_local_ip_valid(ovs_local_ip, vxlan_local_ip):"},{"line_number":191,"context_line":"        return None"},{"line_number":192,"context_line":""},{"line_number":193,"context_line":"    return local_ip"}],"source_content_type":"text/x-python","patch_set":10,"id":"1a930d6b_6cad8e88","line":190,"in_reply_to":"1a930d6b_b0b437f3","updated":"2015-01-21 21:26:21.000000000","message":"It makes no difference to the executing code and I think the way it is now slightly contributes to readability IMO.","commit_id":"537d72c3d20571f0552927675b5c8cc8604e3d19"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"669ebfdd3bd7bbdf2daec7ce45e763903689c0b6","unresolved":false,"context_lines":[{"line_number":187,"context_line":"                      \"and in linuxbridge\u0027s configuration (under the [vxlan] \""},{"line_number":188,"context_line":"                      \"section). if they are both present, they must match.\"))"},{"line_number":189,"context_line":"        return None"},{"line_number":190,"context_line":"    elif not _is_local_ip_valid(ovs_local_ip, vxlan_local_ip):"},{"line_number":191,"context_line":"        return None"},{"line_number":192,"context_line":""},{"line_number":193,"context_line":"    return local_ip"}],"source_content_type":"text/x-python","patch_set":10,"id":"1a930d6b_5dc7b912","line":190,"in_reply_to":"3a961159_58787c2c","updated":"2015-01-21 09:39:13.000000000","message":"Actually think that \u0027elif\u0027 is clearer, since there\u0027s no way that this will be True of the the first one was also True (since that code returns).","commit_id":"537d72c3d20571f0552927675b5c8cc8604e3d19"},{"author":{"_account_id":261,"name":"Salvatore Orlando","email":"salv.orlando@gmail.com","username":"salvatore-orlando"},"change_message_id":"687038f2e13171a9f2179bfb03083b32853643bf","unresolved":false,"context_lines":[{"line_number":35,"context_line":"TUNNEL_OVERHEAD \u003d {"},{"line_number":36,"context_line":"    \u0027gre\u0027: 24,"},{"line_number":37,"context_line":"    \u0027vxlan\u0027: 50,"},{"line_number":38,"context_line":"}"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"# The MTU most machines start with. Does not indicate the effective default MTU"},{"line_number":41,"context_line":"# the machine might actually default to!"}],"source_content_type":"text/x-python","patch_set":11,"id":"da86d52c_2519f322","line":38,"updated":"2015-02-08 20:54:20.000000000","message":"I though I posted this a long time ago, but then found out I had draft comments for about 10 weeks...\n\nThese are the calculations I did together with the OVS folks. (well, they did those, my part was just saying \u0027thank you\u0027).\n\nI don\u0027t know where the 24 number is coming from for GRE honestly, maybe there\u0027s something I\u0027m missing.\n\nGRE: Eth (14) + IP (20) + GRE (4) + Key (4) \u003d 42\n\nVXLAN: Eth (14) + IP (20) + UDP (8) + VXLAN (8) \u003d 50\n\nWhile restricting the MTU for STT is not strictly necessary for TCP connections, it might be useful for UDP connections. Here\u0027s the STT overhead:\n\nSTT: Eth (14) + IP (20) + TCP (20) + STT (18) \u003d 72","commit_id":"f8d1e2f5600c7945eb61de09640d60c1ca51cf03"},{"author":{"_account_id":6698,"name":"Robert Pothier","email":"robpothier35@gmail.com","username":"rpothier"},"change_message_id":"c759b413ea4ec37844854e42c4fbf40f684e78fc","unresolved":false,"context_lines":[{"line_number":35,"context_line":"TUNNEL_OVERHEAD \u003d {"},{"line_number":36,"context_line":"    \u0027gre\u0027: 24,"},{"line_number":37,"context_line":"    \u0027vxlan\u0027: 50,"},{"line_number":38,"context_line":"}"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"# The MTU most machines start with. Does not indicate the effective default MTU"},{"line_number":41,"context_line":"# the machine might actually default to!"}],"source_content_type":"text/x-python","patch_set":11,"id":"ba7be1f8_4082ab00","line":38,"updated":"2015-02-25 19:08:04.000000000","message":"Please move this to a common area so it can be shared.","commit_id":"f8d1e2f5600c7945eb61de09640d60c1ca51cf03"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"50d84bfe0a62b616e269bd71140774330086c5d9","unresolved":false,"context_lines":[{"line_number":35,"context_line":"TUNNEL_OVERHEAD \u003d {"},{"line_number":36,"context_line":"    \u0027gre\u0027: 24,"},{"line_number":37,"context_line":"    \u0027vxlan\u0027: 50,"},{"line_number":38,"context_line":"}"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"# The MTU most machines start with. Does not indicate the effective default MTU"},{"line_number":41,"context_line":"# the machine might actually default to!"}],"source_content_type":"text/x-python","patch_set":11,"id":"ba7be1f8_86905844","line":38,"in_reply_to":"ba7be1f8_4082ab00","updated":"2015-03-03 16:29:15.000000000","message":"Salvatore, 10 weeks is nothing :) thanks!\n\nRob - I don\u0027t see the point of moving it outside. Whoever needs it next can move it itself (just a dict) and if not an \u0027import neutron.cmd.sanity.checks\u0027 can always be added.","commit_id":"f8d1e2f5600c7945eb61de09640d60c1ca51cf03"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"ed9b4e921190e01fe1c297ab573a4fff32da7187","unresolved":false,"context_lines":[{"line_number":39,"context_line":""},{"line_number":40,"context_line":"# The MTU most machines start with. Does not indicate the effective default MTU"},{"line_number":41,"context_line":"# the machine might actually default to!"},{"line_number":42,"context_line":"DEFAULT_MTU \u003d 1500"},{"line_number":43,"context_line":""},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"def ovs_vxlan_supported(root_helper, from_ip\u003d\u0027192.0.2.1\u0027, to_ip\u003d\u0027192.0.2.2\u0027):"}],"source_content_type":"text/x-python","patch_set":11,"id":"1a930d6b_921ade6c","line":42,"updated":"2015-01-21 16:06:36.000000000","message":"why not use the MTU defined int he following configuration file\n\n# (IntOpt) This is the MTU size of veth interfaces.\n# Do not change unless you have a good reason to.\n# The default MTU size of veth interfaces is 1500.\n# This option has no effect if use_veth_interconnection is False\n# veth_mtu \u003d\n# Example: veth_mtu \u003d 1504","commit_id":"f8d1e2f5600c7945eb61de09640d60c1ca51cf03"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"1bba4f04c2cb5389a7edea2fc2e549db376d939a","unresolved":false,"context_lines":[{"line_number":39,"context_line":""},{"line_number":40,"context_line":"# The MTU most machines start with. Does not indicate the effective default MTU"},{"line_number":41,"context_line":"# the machine might actually default to!"},{"line_number":42,"context_line":"DEFAULT_MTU \u003d 1500"},{"line_number":43,"context_line":""},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"def ovs_vxlan_supported(root_helper, from_ip\u003d\u0027192.0.2.1\u0027, to_ip\u003d\u0027192.0.2.2\u0027):"}],"source_content_type":"text/x-python","patch_set":11,"id":"1a930d6b_2c546653","line":42,"in_reply_to":"1a930d6b_5e6a3d1b","updated":"2015-01-21 21:42:57.000000000","message":"I will extend on this point and clarify that there is another MTU option in neutron (whose exact name eludes me atm), which configures the MTU of other Neutron interfaces (l3, dhcp...), and also another one in Nova which determines the MTU of the tap devices connected to the instances (iirc). \n\nThose are all quite a mess and don\u0027t work well, which is why Ian is working on his MTU spec for the next release. In the mean time, this serves as somewhat of a sanity check to make sure that, even if the exact MTU figure the user needs to configure is incorrect, the user is still aware of this issue (granted that he must run the sanity checks with the --check_mtu flag on first).","commit_id":"f8d1e2f5600c7945eb61de09640d60c1ca51cf03"},{"author":{"_account_id":3217,"name":"Ian Wells","username":"ijw-ubuntu"},"change_message_id":"de755ecf3c62a8b054c45f68fe235f270550993f","unresolved":false,"context_lines":[{"line_number":39,"context_line":""},{"line_number":40,"context_line":"# The MTU most machines start with. Does not indicate the effective default MTU"},{"line_number":41,"context_line":"# the machine might actually default to!"},{"line_number":42,"context_line":"DEFAULT_MTU \u003d 1500"},{"line_number":43,"context_line":""},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"def ovs_vxlan_supported(root_helper, from_ip\u003d\u0027192.0.2.1\u0027, to_ip\u003d\u0027192.0.2.2\u0027):"}],"source_content_type":"text/x-python","patch_set":11,"id":"1a930d6b_5e6a3d1b","line":42,"in_reply_to":"1a930d6b_921ade6c","updated":"2015-01-21 17:39:22.000000000","message":"Because it\u0027s the veth_mtu, and this is the MTU set on veth interfaces.  It can be wrong.  Assaf is testing an actual MTU in use on an interface that passes the packets, which is one that definitely applies, and I agree with his approach.","commit_id":"f8d1e2f5600c7945eb61de09640d60c1ca51cf03"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"ed9b4e921190e01fe1c297ab573a4fff32da7187","unresolved":false,"context_lines":[{"line_number":187,"context_line":"                      \"openvswitch\u0027s configuration (under the [ovs] section) \""},{"line_number":188,"context_line":"                      \"and in linuxbridge\u0027s configuration (under the [vxlan] \""},{"line_number":189,"context_line":"                      \"section). if they are both present, they must match.\"))"},{"line_number":190,"context_line":"        return None"},{"line_number":191,"context_line":"    elif not _is_local_ip_valid():"},{"line_number":192,"context_line":"        return None"},{"line_number":193,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"1a930d6b_723492de","line":190,"updated":"2015-01-21 16:06:36.000000000","message":"you do not need return None. If you do return then by default None is returned","commit_id":"f8d1e2f5600c7945eb61de09640d60c1ca51cf03"},{"author":{"_account_id":261,"name":"Salvatore Orlando","email":"salv.orlando@gmail.com","username":"salvatore-orlando"},"change_message_id":"687038f2e13171a9f2179bfb03083b32853643bf","unresolved":false,"context_lines":[{"line_number":187,"context_line":"                      \"openvswitch\u0027s configuration (under the [ovs] section) \""},{"line_number":188,"context_line":"                      \"and in linuxbridge\u0027s configuration (under the [vxlan] \""},{"line_number":189,"context_line":"                      \"section). if they are both present, they must match.\"))"},{"line_number":190,"context_line":"        return None"},{"line_number":191,"context_line":"    elif not _is_local_ip_valid():"},{"line_number":192,"context_line":"        return None"},{"line_number":193,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"da86d52c_45ed07ef","line":190,"in_reply_to":"1a930d6b_6cdbce1b","updated":"2015-02-08 20:54:20.000000000","message":"I have no interest in sanctioning whether it\u0027s better to use \u0027return\u0027 or \u0027return None\u0027. However I am interested in your comment regarding the return values of this method. It sounds like in this case False is different from None. I can\u0027t find out why however, because as you say in the docstring it either returns an IP or None.","commit_id":"f8d1e2f5600c7945eb61de09640d60c1ca51cf03"},{"author":{"_account_id":3217,"name":"Ian Wells","username":"ijw-ubuntu"},"change_message_id":"de755ecf3c62a8b054c45f68fe235f270550993f","unresolved":false,"context_lines":[{"line_number":187,"context_line":"                      \"openvswitch\u0027s configuration (under the [ovs] section) \""},{"line_number":188,"context_line":"                      \"and in linuxbridge\u0027s configuration (under the [vxlan] \""},{"line_number":189,"context_line":"                      \"section). if they are both present, they must match.\"))"},{"line_number":190,"context_line":"        return None"},{"line_number":191,"context_line":"    elif not _is_local_ip_valid():"},{"line_number":192,"context_line":"        return None"},{"line_number":193,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"1a930d6b_fe8209ff","line":190,"in_reply_to":"1a930d6b_723492de","updated":"2015-01-21 17:39:22.000000000","message":"I like it explicit, actually.  Better than \u0027return\u0027 which has the implication of \u0027because I forgot this function had a return value\u0027.","commit_id":"f8d1e2f5600c7945eb61de09640d60c1ca51cf03"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"1bba4f04c2cb5389a7edea2fc2e549db376d939a","unresolved":false,"context_lines":[{"line_number":187,"context_line":"                      \"openvswitch\u0027s configuration (under the [ovs] section) \""},{"line_number":188,"context_line":"                      \"and in linuxbridge\u0027s configuration (under the [vxlan] \""},{"line_number":189,"context_line":"                      \"section). if they are both present, they must match.\"))"},{"line_number":190,"context_line":"        return None"},{"line_number":191,"context_line":"    elif not _is_local_ip_valid():"},{"line_number":192,"context_line":"        return None"},{"line_number":193,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"1a930d6b_6cdbce1b","line":190,"in_reply_to":"1a930d6b_fe8209ff","updated":"2015-01-21 21:42:57.000000000","message":"Likewise - explicit is better than implicit. Also in this patch the actual \u0027None/False vs True/not-False\u0027 matters - see the previous patchset and the comments made by Terry, so I\u0027d like to be extra explicit here.","commit_id":"f8d1e2f5600c7945eb61de09640d60c1ca51cf03"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"50d84bfe0a62b616e269bd71140774330086c5d9","unresolved":false,"context_lines":[{"line_number":187,"context_line":"                      \"openvswitch\u0027s configuration (under the [ovs] section) \""},{"line_number":188,"context_line":"                      \"and in linuxbridge\u0027s configuration (under the [vxlan] \""},{"line_number":189,"context_line":"                      \"section). if they are both present, they must match.\"))"},{"line_number":190,"context_line":"        return None"},{"line_number":191,"context_line":"    elif not _is_local_ip_valid():"},{"line_number":192,"context_line":"        return None"},{"line_number":193,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"ba7be1f8_6648c4b3","line":190,"in_reply_to":"ba7be1f8_553059e8","updated":"2015-03-03 16:29:15.000000000","message":"lol Kevin.\n\nEither way I can\u0027t remember why I wrote down \u0027None/False\u0027 when this code indeed doesn\u0027t return \u0027False\u0027. So I\u0027m leaving this as is.","commit_id":"f8d1e2f5600c7945eb61de09640d60c1ca51cf03"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"3c0af39cf85f0103ca9e271cd8d291e226ec8e12","unresolved":false,"context_lines":[{"line_number":187,"context_line":"                      \"openvswitch\u0027s configuration (under the [ovs] section) \""},{"line_number":188,"context_line":"                      \"and in linuxbridge\u0027s configuration (under the [vxlan] \""},{"line_number":189,"context_line":"                      \"section). if they are both present, they must match.\"))"},{"line_number":190,"context_line":"        return None"},{"line_number":191,"context_line":"    elif not _is_local_ip_valid():"},{"line_number":192,"context_line":"        return None"},{"line_number":193,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"ba7be1f8_553059e8","line":190,"in_reply_to":"da86d52c_45ed07ef","updated":"2015-02-27 09:30:46.000000000","message":"well I prefer the following:\n\nreturn None if not None else not None","commit_id":"f8d1e2f5600c7945eb61de09640d60c1ca51cf03"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"ed9b4e921190e01fe1c297ab573a4fff32da7187","unresolved":false,"context_lines":[{"line_number":189,"context_line":"                      \"section). if they are both present, they must match.\"))"},{"line_number":190,"context_line":"        return None"},{"line_number":191,"context_line":"    elif not _is_local_ip_valid():"},{"line_number":192,"context_line":"        return None"},{"line_number":193,"context_line":""},{"line_number":194,"context_line":"    return local_ip"},{"line_number":195,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"1a930d6b_9231feee","line":192,"updated":"2015-01-21 16:06:36.000000000","message":"ditto","commit_id":"f8d1e2f5600c7945eb61de09640d60c1ca51cf03"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"ed9b4e921190e01fe1c297ab573a4fff32da7187","unresolved":false,"context_lines":[{"line_number":197,"context_line":"def _get_mtu_of_ip(local_ip):"},{"line_number":198,"context_line":"    args \u003d [\u0027ip\u0027, \u0027addr\u0027, \u0027show\u0027, \u0027to\u0027, local_ip]"},{"line_number":199,"context_line":"    output \u003d agent_utils.execute(args)"},{"line_number":200,"context_line":"    match \u003d re.search(r\u0027mtu (?P\u003cmtu\u003e\\d+)\u0027, output)"},{"line_number":201,"context_line":""},{"line_number":202,"context_line":"    if not match:"},{"line_number":203,"context_line":"        LOG.warning(_LW(\u0027Could not discover mtu of device owning %s.\u0027),"}],"source_content_type":"text/x-python","patch_set":11,"id":"1a930d6b_12d0ae4f","line":200,"updated":"2015-01-21 16:06:36.000000000","message":"do we know which interface we are using? If so then we can read from:\n/sys/devices/virtual/net/\u003cinterface\u003e/mtu","commit_id":"f8d1e2f5600c7945eb61de09640d60c1ca51cf03"},{"author":{"_account_id":3217,"name":"Ian Wells","username":"ijw-ubuntu"},"change_message_id":"de755ecf3c62a8b054c45f68fe235f270550993f","unresolved":false,"context_lines":[{"line_number":197,"context_line":"def _get_mtu_of_ip(local_ip):"},{"line_number":198,"context_line":"    args \u003d [\u0027ip\u0027, \u0027addr\u0027, \u0027show\u0027, \u0027to\u0027, local_ip]"},{"line_number":199,"context_line":"    output \u003d agent_utils.execute(args)"},{"line_number":200,"context_line":"    match \u003d re.search(r\u0027mtu (?P\u003cmtu\u003e\\d+)\u0027, output)"},{"line_number":201,"context_line":""},{"line_number":202,"context_line":"    if not match:"},{"line_number":203,"context_line":"        LOG.warning(_LW(\u0027Could not discover mtu of device owning %s.\u0027),"}],"source_content_type":"text/x-python","patch_set":11,"id":"1a930d6b_de30ed58","line":200,"in_reply_to":"1a930d6b_12d0ae4f","updated":"2015-01-21 17:39:22.000000000","message":"We may not be using one interface, because there may be multiple routes to the other host with different metrics.  But Assaf\u0027s usage here is better than the one Gary is proposing - the route table is used to find the currently active interface.\n\nMore to the point, this tests local network MTU  but can\u0027t test the path MTU (that is in fact impossible since the path is not fixed).If this MTU is too low we clearly do have a problem, so that\u0027s probably fine for a sanity, but it\u0027s not absolute proof the packet won\u0027t fragment.\n\nThe only thing I would debate is the name of the function: _get_mtu_of_ip absolutely misleads the reader because of the two points made above.  Change it or at least add a docstring.","commit_id":"f8d1e2f5600c7945eb61de09640d60c1ca51cf03"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"50d84bfe0a62b616e269bd71140774330086c5d9","unresolved":false,"context_lines":[{"line_number":197,"context_line":"def _get_mtu_of_ip(local_ip):"},{"line_number":198,"context_line":"    args \u003d [\u0027ip\u0027, \u0027addr\u0027, \u0027show\u0027, \u0027to\u0027, local_ip]"},{"line_number":199,"context_line":"    output \u003d agent_utils.execute(args)"},{"line_number":200,"context_line":"    match \u003d re.search(r\u0027mtu (?P\u003cmtu\u003e\\d+)\u0027, output)"},{"line_number":201,"context_line":""},{"line_number":202,"context_line":"    if not match:"},{"line_number":203,"context_line":"        LOG.warning(_LW(\u0027Could not discover mtu of device owning %s.\u0027),"}],"source_content_type":"text/x-python","patch_set":11,"id":"ba7be1f8_8664d822","line":200,"in_reply_to":"1a930d6b_2cf8e67b","updated":"2015-03-03 16:29:15.000000000","message":"I looked into the folder issue. My setup doesn\u0027t contain such a folder for eth0 for some reason, so I\u0027m leaving the code as is.","commit_id":"f8d1e2f5600c7945eb61de09640d60c1ca51cf03"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"1bba4f04c2cb5389a7edea2fc2e549db376d939a","unresolved":false,"context_lines":[{"line_number":197,"context_line":"def _get_mtu_of_ip(local_ip):"},{"line_number":198,"context_line":"    args \u003d [\u0027ip\u0027, \u0027addr\u0027, \u0027show\u0027, \u0027to\u0027, local_ip]"},{"line_number":199,"context_line":"    output \u003d agent_utils.execute(args)"},{"line_number":200,"context_line":"    match \u003d re.search(r\u0027mtu (?P\u003cmtu\u003e\\d+)\u0027, output)"},{"line_number":201,"context_line":""},{"line_number":202,"context_line":"    if not match:"},{"line_number":203,"context_line":"        LOG.warning(_LW(\u0027Could not discover mtu of device owning %s.\u0027),"}],"source_content_type":"text/x-python","patch_set":11,"id":"1a930d6b_2cf8e67b","line":200,"in_reply_to":"1a930d6b_de30ed58","updated":"2015-01-21 21:42:57.000000000","message":"Ian, can you clarify what you mean re: \"Assaf\u0027s usage here\"? I probably missed out on a couple of talks you guys had :)\n\nI accept the point regarding the naming, I\u0027ll reword it in the next patchset. \n\nWill also look into /sys/devices/virtual/net/\u003cinterface\u003e/mtu as it\u0027s probably better than doing this :)","commit_id":"f8d1e2f5600c7945eb61de09640d60c1ca51cf03"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"ed9b4e921190e01fe1c297ab573a4fff32da7187","unresolved":false,"context_lines":[{"line_number":202,"context_line":"    if not match:"},{"line_number":203,"context_line":"        LOG.warning(_LW(\u0027Could not discover mtu of device owning %s.\u0027),"},{"line_number":204,"context_line":"                    local_ip)"},{"line_number":205,"context_line":"        return None"},{"line_number":206,"context_line":"    return int(match.group(\u0027mtu\u0027))"},{"line_number":207,"context_line":""},{"line_number":208,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"1a930d6b_125d0e22","line":205,"updated":"2015-01-21 16:06:36.000000000","message":"ditto","commit_id":"f8d1e2f5600c7945eb61de09640d60c1ca51cf03"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"746cb135f6e43bb5adcd13de3ece0e65484f6cb9","unresolved":false,"context_lines":[{"line_number":16,"context_line":"import re"},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"import netaddr"},{"line_number":19,"context_line":"from oslo.config import cfg"},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"from neutron.agent.linux import ip_lib"},{"line_number":22,"context_line":"from neutron.agent.linux import ip_link_support"}],"source_content_type":"text/x-python","patch_set":12,"id":"9a80dd14_acddd044","line":19,"updated":"2015-03-04 18:10:30.000000000","message":"Thsi should change to using oslo_config","commit_id":"d9047827275bc7387c295d25a83b868a8b900d09"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"105e386a0ae2ac297b1c2ec52dab656eb52773b5","unresolved":false,"context_lines":[{"line_number":16,"context_line":"import re"},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"import netaddr"},{"line_number":19,"context_line":"from oslo.config import cfg"},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"from neutron.agent.linux import ip_lib"},{"line_number":22,"context_line":"from neutron.agent.linux import ip_link_support"}],"source_content_type":"text/x-python","patch_set":12,"id":"9a80dd14_beeebaea","line":19,"in_reply_to":"9a80dd14_acddd044","updated":"2015-03-05 06:18:11.000000000","message":"Done","commit_id":"d9047827275bc7387c295d25a83b868a8b900d09"},{"author":{"_account_id":2874,"name":"yong sheng gong","email":"gong.yongsheng@99cloud.net","username":"gongysh"},"change_message_id":"9ea8cb62efb8711e110bf74548dd74d8d7c23202","unresolved":false,"context_lines":[{"line_number":241,"context_line":"                        \u0027at least %(needed_mtu)s for the configured tunnel \u0027"},{"line_number":242,"context_line":"                        \u0027types (the tunnel types currently configured are: \u0027"},{"line_number":243,"context_line":"                        \u0027%(tunnel_types)r and the maximum overhead is \u0027"},{"line_number":244,"context_line":"                        \u0027%(max_overhead)s)!\u0027),"},{"line_number":245,"context_line":"                    {\u0027local_ip\u0027: local_ip,"},{"line_number":246,"context_line":"                     \u0027current_mtu\u0027: current_mtu,"},{"line_number":247,"context_line":"                     \u0027needed_mtu\u0027: needed_mtu,"}],"source_content_type":"text/x-python","patch_set":13,"id":"9a80dd14_f7bbf5c1","line":244,"updated":"2015-03-05 07:51:53.000000000","message":"s) -\u003e s","commit_id":"0cd5f423e927f9892ca4d6917e1f7056f07b17a2"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"f9c76baec8b3b69e19346e26a0c6ca0c04fa4df3","unresolved":false,"context_lines":[{"line_number":241,"context_line":"                        \u0027at least %(needed_mtu)s for the configured tunnel \u0027"},{"line_number":242,"context_line":"                        \u0027types (the tunnel types currently configured are: \u0027"},{"line_number":243,"context_line":"                        \u0027%(tunnel_types)r and the maximum overhead is \u0027"},{"line_number":244,"context_line":"                        \u0027%(max_overhead)s)!\u0027),"},{"line_number":245,"context_line":"                    {\u0027local_ip\u0027: local_ip,"},{"line_number":246,"context_line":"                     \u0027current_mtu\u0027: current_mtu,"},{"line_number":247,"context_line":"                     \u0027needed_mtu\u0027: needed_mtu,"}],"source_content_type":"text/x-python","patch_set":13,"id":"9a80dd14_28128cb1","line":244,"in_reply_to":"9a80dd14_f7bbf5c1","updated":"2015-03-05 09:04:16.000000000","message":"Actually the \u0027)\u0027 closes the \u0027(\u0027 that was opened in line 242, so it should be there.","commit_id":"0cd5f423e927f9892ca4d6917e1f7056f07b17a2"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"a02c2984b8b2414f4b57de0deb318559d54a9ecc","unresolved":false,"context_lines":[{"line_number":183,"context_line":"        if ovs_local_ip \u003d\u003d vxlan_local_ip:"},{"line_number":184,"context_line":"            return True"},{"line_number":185,"context_line":"        else:"},{"line_number":186,"context_line":"            LOG.error(_LE(\"Conflicting local_ip configuration found.\"))"},{"line_number":187,"context_line":"    except netaddr.AddrFormatError:"},{"line_number":188,"context_line":"        LOG.error(_LE(\"Cannot parse one of the local_ip options configured.\"))"},{"line_number":189,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"9a80dd14_4b905a95","line":186,"updated":"2015-03-12 18:14:20.000000000","message":"why do we have this else and error trace, if we barf at line 188? It sounds redundant.","commit_id":"657be02549d3788066145de92e8da8eb8a4e6ce6"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"1e31dab21babb97fa6e4b31ab428bf8232fed2db","unresolved":false,"context_lines":[{"line_number":183,"context_line":"        if ovs_local_ip \u003d\u003d vxlan_local_ip:"},{"line_number":184,"context_line":"            return True"},{"line_number":185,"context_line":"        else:"},{"line_number":186,"context_line":"            LOG.error(_LE(\"Conflicting local_ip configuration found.\"))"},{"line_number":187,"context_line":"    except netaddr.AddrFormatError:"},{"line_number":188,"context_line":"        LOG.error(_LE(\"Cannot parse one of the local_ip options configured.\"))"},{"line_number":189,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"9a80dd14_54d2db5b","line":186,"in_reply_to":"9a80dd14_4b905a95","updated":"2015-03-18 09:52:45.000000000","message":"These are 2 different errors (so the messages should be different IMO).","commit_id":"657be02549d3788066145de92e8da8eb8a4e6ce6"},{"author":{"_account_id":10370,"name":"Romil Gupta","email":"romilgupta19@gmail.com","username":"romilg"},"change_message_id":"0ab2d4971c56fc54c36f69faecb03955522267d4","unresolved":false,"context_lines":[{"line_number":199,"context_line":""},{"line_number":200,"context_line":"    \u0027local_ip\u0027 is configured in the OVS (for openvswitch) and in the VXLAN (for"},{"line_number":201,"context_line":"    linuxbridge) sections. If at least one exists, returns it (if both exists,"},{"line_number":202,"context_line":"    make sure that they match). If some inconsistency was found - returns None."},{"line_number":203,"context_line":"    \"\"\""},{"line_number":204,"context_line":"    local_ip \u003d cfg.CONF.OVS.local_ip or cfg.CONF.VXLAN.local_ip"},{"line_number":205,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"9a80dd14_59869466","line":202,"updated":"2015-03-05 17:57:45.000000000","message":"I have a concern with this statement \"....(if both exists, make sure that they match). If some inconsistency was found - returns None. \"\n\nCould you please clarify why we need to force this. Also, in which scenario we need to run multiple L2 Agent\u0027s on a single machine.\n\nIf it\u0027s a valid scenario, then I may be having OVS.local_ip as eth2(gre)\nand Linuxbridge VXLAN.local_ip as eth3(vxlan)\n\nwhere eth2 and eth3 are separate data networks.","commit_id":"657be02549d3788066145de92e8da8eb8a4e6ce6"},{"author":{"_account_id":10370,"name":"Romil Gupta","email":"romilgupta19@gmail.com","username":"romilg"},"change_message_id":"68409a544b0f972a3529e06442827e83f2d21e10","unresolved":false,"context_lines":[{"line_number":199,"context_line":""},{"line_number":200,"context_line":"    \u0027local_ip\u0027 is configured in the OVS (for openvswitch) and in the VXLAN (for"},{"line_number":201,"context_line":"    linuxbridge) sections. If at least one exists, returns it (if both exists,"},{"line_number":202,"context_line":"    make sure that they match). If some inconsistency was found - returns None."},{"line_number":203,"context_line":"    \"\"\""},{"line_number":204,"context_line":"    local_ip \u003d cfg.CONF.OVS.local_ip or cfg.CONF.VXLAN.local_ip"},{"line_number":205,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"9a80dd14_7aa91323","line":202,"in_reply_to":"9a80dd14_1aae5798","updated":"2015-03-09 17:38:57.000000000","message":"Also, eventually, this \u0027_is_local_ip_valid()\u0027  method will be redundant when this patchset gets merged.\n\n   Check for valid local_ip for OVS tunnel\n   https://review.openstack.org/#/c/154043/\n\nAs far as I know we already have such kind of check for Linux bridge Agent.","commit_id":"657be02549d3788066145de92e8da8eb8a4e6ce6"},{"author":{"_account_id":10370,"name":"Romil Gupta","email":"romilgupta19@gmail.com","username":"romilg"},"change_message_id":"8671e2616cab4e614ed14f3d2e53ffd57882c0ab","unresolved":false,"context_lines":[{"line_number":199,"context_line":""},{"line_number":200,"context_line":"    \u0027local_ip\u0027 is configured in the OVS (for openvswitch) and in the VXLAN (for"},{"line_number":201,"context_line":"    linuxbridge) sections. If at least one exists, returns it (if both exists,"},{"line_number":202,"context_line":"    make sure that they match). If some inconsistency was found - returns None."},{"line_number":203,"context_line":"    \"\"\""},{"line_number":204,"context_line":"    local_ip \u003d cfg.CONF.OVS.local_ip or cfg.CONF.VXLAN.local_ip"},{"line_number":205,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"9a80dd14_77768e07","line":202,"in_reply_to":"9a80dd14_3457b459","updated":"2015-03-09 17:11:37.000000000","message":"In that case condition below doesn\u0027t make sense, better remove it.\nif ovs_local_ip \u003d\u003d vxlan_local_ip:\n   return True","commit_id":"657be02549d3788066145de92e8da8eb8a4e6ce6"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"33bdf7b42a916660c98671f71aa603dc05825609","unresolved":false,"context_lines":[{"line_number":199,"context_line":""},{"line_number":200,"context_line":"    \u0027local_ip\u0027 is configured in the OVS (for openvswitch) and in the VXLAN (for"},{"line_number":201,"context_line":"    linuxbridge) sections. If at least one exists, returns it (if both exists,"},{"line_number":202,"context_line":"    make sure that they match). If some inconsistency was found - returns None."},{"line_number":203,"context_line":"    \"\"\""},{"line_number":204,"context_line":"    local_ip \u003d cfg.CONF.OVS.local_ip or cfg.CONF.VXLAN.local_ip"},{"line_number":205,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"9a80dd14_b18bb61d","line":202,"in_reply_to":"9a80dd14_525a7d83","updated":"2015-03-09 16:09:10.000000000","message":"I think I understand your question now. The reason I inserted this check was to make sure the code checks the correct interface - if 2 different local_ip are entered, I didn\u0027t want to start thinking about which one is directed at which interface, etc. Since the usecase of having more than one L2 agents is unlikely, this doesn\u0027t really conflict (and forcing both options to be identical will also help prevent user-made errors).\n\nBy the way, I would imagine that a usecase of someone deploying multiple L2 agents will also include multiple ml2_conf.ini files - one for each agent, thus the sanity check can be made to run against a specific set of ml2_conf.ini file and check the correct data, thus this code is still valid.\n\nIf all else fails and it turns out that this logic is not sufficient, I think it can be changed in a followup patch. Does that make sense?","commit_id":"657be02549d3788066145de92e8da8eb8a4e6ce6"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"abac4cd7af019941e7ae95ac21520e89df4fcf61","unresolved":false,"context_lines":[{"line_number":199,"context_line":""},{"line_number":200,"context_line":"    \u0027local_ip\u0027 is configured in the OVS (for openvswitch) and in the VXLAN (for"},{"line_number":201,"context_line":"    linuxbridge) sections. If at least one exists, returns it (if both exists,"},{"line_number":202,"context_line":"    make sure that they match). If some inconsistency was found - returns None."},{"line_number":203,"context_line":"    \"\"\""},{"line_number":204,"context_line":"    local_ip \u003d cfg.CONF.OVS.local_ip or cfg.CONF.VXLAN.local_ip"},{"line_number":205,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"9a80dd14_817ae9ba","line":202,"in_reply_to":"9a80dd14_59869466","updated":"2015-03-09 14:23:03.000000000","message":"I\u0027m not entirely sure that the situation you described (running multiple, different-typed L2 agents, for example one OVS and another linuxbridge) is even possible, since they will probably collide (for the record, a while back I added patches specifically to allow support for multiple OVS agents to run, and it was no picnic).\n\nMoreover, I didn\u0027t understand from your comment if you know for a fact if it is possible or not. So unless we do get someone to validate that logic for this specific case should be inserted, I don\u0027t see why a manual, CLI checking script should cover this case.","commit_id":"657be02549d3788066145de92e8da8eb8a4e6ce6"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"fc2d68f1d90d338bc76c3291fdbf73b4853b7e2c","unresolved":false,"context_lines":[{"line_number":199,"context_line":""},{"line_number":200,"context_line":"    \u0027local_ip\u0027 is configured in the OVS (for openvswitch) and in the VXLAN (for"},{"line_number":201,"context_line":"    linuxbridge) sections. If at least one exists, returns it (if both exists,"},{"line_number":202,"context_line":"    make sure that they match). If some inconsistency was found - returns None."},{"line_number":203,"context_line":"    \"\"\""},{"line_number":204,"context_line":"    local_ip \u003d cfg.CONF.OVS.local_ip or cfg.CONF.VXLAN.local_ip"},{"line_number":205,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"9a80dd14_bad38b5e","line":202,"in_reply_to":"9a80dd14_77768e07","updated":"2015-03-09 17:20:49.000000000","message":"I disagree - it makes sense - this code would like to be extra cautious and check the MTU *only* when it knows which IP to check. If there are 2 different local_ips, how can it know which one to check?","commit_id":"657be02549d3788066145de92e8da8eb8a4e6ce6"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"dd1724ef5b1911462c2822f4551b9921fdb79200","unresolved":false,"context_lines":[{"line_number":199,"context_line":""},{"line_number":200,"context_line":"    \u0027local_ip\u0027 is configured in the OVS (for openvswitch) and in the VXLAN (for"},{"line_number":201,"context_line":"    linuxbridge) sections. If at least one exists, returns it (if both exists,"},{"line_number":202,"context_line":"    make sure that they match). If some inconsistency was found - returns None."},{"line_number":203,"context_line":"    \"\"\""},{"line_number":204,"context_line":"    local_ip \u003d cfg.CONF.OVS.local_ip or cfg.CONF.VXLAN.local_ip"},{"line_number":205,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"9a80dd14_c5f4e45c","line":202,"in_reply_to":"9a80dd14_7aa91323","updated":"2015-03-09 18:07:22.000000000","message":"I\u0027m fine with the code being redundant later on - it will need another revision probably when the MTU patches land.\n\nre: \"check both\" - I\u0027ll think about it and reply back in the next few days. I still don\u0027t think this is really needed as it will probably only add confusion to a simple sanity check. for example, what if one of the IPs has a typo (10.35.6.13 instead of 10.35.6.12 for example) and now one of the IPs point to an unknown interface? should this raise an error or not? if so, what\u0027s the error? that the interface couldn\u0027t be found? maybe it\u0027s on a different namespace ... ... ... And what if it points to an actual interface whose MTU is valid, but it isn\u0027t used at all and a false-positive is given?\n\nCan you see my concerns here? I don\u0027t think the code complication is worth it for a relatively simple sanity check, especially since we don\u0027t even know if such a case is even possible in current deployment setups.","commit_id":"657be02549d3788066145de92e8da8eb8a4e6ce6"},{"author":{"_account_id":10370,"name":"Romil Gupta","email":"romilgupta19@gmail.com","username":"romilg"},"change_message_id":"58650fde948a5e8d2ec963541c050ad74ad4594a","unresolved":false,"context_lines":[{"line_number":199,"context_line":""},{"line_number":200,"context_line":"    \u0027local_ip\u0027 is configured in the OVS (for openvswitch) and in the VXLAN (for"},{"line_number":201,"context_line":"    linuxbridge) sections. If at least one exists, returns it (if both exists,"},{"line_number":202,"context_line":"    make sure that they match). If some inconsistency was found - returns None."},{"line_number":203,"context_line":"    \"\"\""},{"line_number":204,"context_line":"    local_ip \u003d cfg.CONF.OVS.local_ip or cfg.CONF.VXLAN.local_ip"},{"line_number":205,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"9a80dd14_525a7d83","line":202,"in_reply_to":"9a80dd14_817ae9ba","updated":"2015-03-09 15:36:37.000000000","message":"Maybe my question was not that much clear to you in my previous comment, let me ask this way:\n\nSince, we don\u0027t have a clarity about a use case (running multiple, different-typed L2 agents, for example, one OVS and another Linux Bridge) on a single machine then why we are quoting this message:\n\n\"\"\"  ....(if both exists, make sure that they match). If some inconsistency was found - returns None. \"\n\nWhy do we have this condition in the code, that local_ip for both should match?\n\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\n\nNow let\u0027s assume we find a valid use case to have multiple L2 agents on a single machine, in that case, why we should restrict a user to assign same local_ip to both OVS and LB. In a deployment, it may possible that both agents configured with different tunnel_type uses separate data networks (eth2(vxlan) and eth3(gre)) in that case local_ip would be different.","commit_id":"657be02549d3788066145de92e8da8eb8a4e6ce6"},{"author":{"_account_id":10370,"name":"Romil Gupta","email":"romilgupta19@gmail.com","username":"romilg"},"change_message_id":"1180f59d410935c56322178a64f2d8a53eeae691","unresolved":false,"context_lines":[{"line_number":199,"context_line":""},{"line_number":200,"context_line":"    \u0027local_ip\u0027 is configured in the OVS (for openvswitch) and in the VXLAN (for"},{"line_number":201,"context_line":"    linuxbridge) sections. If at least one exists, returns it (if both exists,"},{"line_number":202,"context_line":"    make sure that they match). If some inconsistency was found - returns None."},{"line_number":203,"context_line":"    \"\"\""},{"line_number":204,"context_line":"    local_ip \u003d cfg.CONF.OVS.local_ip or cfg.CONF.VXLAN.local_ip"},{"line_number":205,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"9a80dd14_d1690afa","line":202,"in_reply_to":"9a80dd14_b18bb61d","updated":"2015-03-09 16:29:23.000000000","message":"It doesn\u0027t include multiple ml2_conf.ini files even a single ml2_conf.ini is enough to configure both agents. All we need to set it this way:\n\n  [ovs]\n  local_ip\u003dx.x.x.x\n\n  [vxlan]\n  local_ip\u003dy.y.y.y\n\nReferences:\n\nhttp://docs.openstack.org/trunk/config-reference/content/networking-plugin-linuxbridge_agent.html\n\nhttp://docs.openstack.org/trunk/config-reference/content/networking-plugin-openvswitch_agent.html","commit_id":"657be02549d3788066145de92e8da8eb8a4e6ce6"},{"author":{"_account_id":10370,"name":"Romil Gupta","email":"romilgupta19@gmail.com","username":"romilg"},"change_message_id":"51a42c18a158ae25da2e795a9bb35e7119e68496","unresolved":false,"context_lines":[{"line_number":199,"context_line":""},{"line_number":200,"context_line":"    \u0027local_ip\u0027 is configured in the OVS (for openvswitch) and in the VXLAN (for"},{"line_number":201,"context_line":"    linuxbridge) sections. If at least one exists, returns it (if both exists,"},{"line_number":202,"context_line":"    make sure that they match). If some inconsistency was found - returns None."},{"line_number":203,"context_line":"    \"\"\""},{"line_number":204,"context_line":"    local_ip \u003d cfg.CONF.OVS.local_ip or cfg.CONF.VXLAN.local_ip"},{"line_number":205,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"9a80dd14_1aae5798","line":202,"in_reply_to":"9a80dd14_bad38b5e","updated":"2015-03-09 17:27:43.000000000","message":"In that case, I would suggest we should check for both the configured local IP\u0027s.\n\nMake sense?","commit_id":"657be02549d3788066145de92e8da8eb8a4e6ce6"},{"author":{"_account_id":10370,"name":"Romil Gupta","email":"romilgupta19@gmail.com","username":"romilg"},"change_message_id":"71169a14e0c3a852ecad559da80b9308cbadb587","unresolved":false,"context_lines":[{"line_number":199,"context_line":""},{"line_number":200,"context_line":"    \u0027local_ip\u0027 is configured in the OVS (for openvswitch) and in the VXLAN (for"},{"line_number":201,"context_line":"    linuxbridge) sections. If at least one exists, returns it (if both exists,"},{"line_number":202,"context_line":"    make sure that they match). If some inconsistency was found - returns None."},{"line_number":203,"context_line":"    \"\"\""},{"line_number":204,"context_line":"    local_ip \u003d cfg.CONF.OVS.local_ip or cfg.CONF.VXLAN.local_ip"},{"line_number":205,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"9a80dd14_abd6df25","line":202,"in_reply_to":"9a80dd14_c5f4e45c","updated":"2015-03-09 18:45:28.000000000","message":"I understand all your concerns but still  I am not fully convinced. Probably, I will update my score once any core-reviewer approves it and guide me whether it\u0027s a valid use case or not.\n\nDon\u0027t mind :)","commit_id":"657be02549d3788066145de92e8da8eb8a4e6ce6"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"3152d37a23dfd1a19d88e86770b31681cf63c9dd","unresolved":false,"context_lines":[{"line_number":199,"context_line":""},{"line_number":200,"context_line":"    \u0027local_ip\u0027 is configured in the OVS (for openvswitch) and in the VXLAN (for"},{"line_number":201,"context_line":"    linuxbridge) sections. If at least one exists, returns it (if both exists,"},{"line_number":202,"context_line":"    make sure that they match). If some inconsistency was found - returns None."},{"line_number":203,"context_line":"    \"\"\""},{"line_number":204,"context_line":"    local_ip \u003d cfg.CONF.OVS.local_ip or cfg.CONF.VXLAN.local_ip"},{"line_number":205,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"9a80dd14_3457b459","line":202,"in_reply_to":"9a80dd14_d1690afa","updated":"2015-03-09 16:46:23.000000000","message":"I know that technically it\u0027s possible - the question is how likely it is to use one file for 2 agents on the same node in a deployment. This goes back to my first/second post - is it even possible? Won\u0027t the 2 agents conflict?\n\nI think they would (from my various patches to make 2 agents of the same type (ovs) to co-exist on one node), so that\u0027s why I asked if anyone was aware of an actual setup where both ovs and linuxbridge co-exist on one node. If not, that this code doesn\u0027t break anything.\n\nIn conclusion, unless you (or anyone else) know of a deployment which uses both linuxbridge and ovs on the same node, I don\u0027t think this code breaks anything. What do you think?","commit_id":"657be02549d3788066145de92e8da8eb8a4e6ce6"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"a02c2984b8b2414f4b57de0deb318559d54a9ecc","unresolved":false,"context_lines":[{"line_number":201,"context_line":"    linuxbridge) sections. If at least one exists, returns it (if both exists,"},{"line_number":202,"context_line":"    make sure that they match). If some inconsistency was found - returns None."},{"line_number":203,"context_line":"    \"\"\""},{"line_number":204,"context_line":"    local_ip \u003d cfg.CONF.OVS.local_ip or cfg.CONF.VXLAN.local_ip"},{"line_number":205,"context_line":""},{"line_number":206,"context_line":"    if not local_ip:"},{"line_number":207,"context_line":"        LOG.error(_LE(\"The local_ip option must be set in either \""}],"source_content_type":"text/x-python","patch_set":14,"id":"9a80dd14_2bbfa6fd","line":204,"updated":"2015-03-12 18:14:20.000000000","message":"with this you give precedence to local_ip, is this intentional?","commit_id":"657be02549d3788066145de92e8da8eb8a4e6ce6"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"1e31dab21babb97fa6e4b31ab428bf8232fed2db","unresolved":false,"context_lines":[{"line_number":201,"context_line":"    linuxbridge) sections. If at least one exists, returns it (if both exists,"},{"line_number":202,"context_line":"    make sure that they match). If some inconsistency was found - returns None."},{"line_number":203,"context_line":"    \"\"\""},{"line_number":204,"context_line":"    local_ip \u003d cfg.CONF.OVS.local_ip or cfg.CONF.VXLAN.local_ip"},{"line_number":205,"context_line":""},{"line_number":206,"context_line":"    if not local_ip:"},{"line_number":207,"context_line":"        LOG.error(_LE(\"The local_ip option must be set in either \""}],"source_content_type":"text/x-python","patch_set":14,"id":"9a80dd14_945a93f0","line":204,"in_reply_to":"9a80dd14_2bbfa6fd","updated":"2015-03-18 09:52:45.000000000","message":"It doesn\u0027t matter. If they conflict (don\u0027t match) then it will be caught in line 186, and if one of them is empty then the precedence is irrelevant.","commit_id":"657be02549d3788066145de92e8da8eb8a4e6ce6"},{"author":{"_account_id":10370,"name":"Romil Gupta","email":"romilgupta19@gmail.com","username":"romilg"},"change_message_id":"0ab2d4971c56fc54c36f69faecb03955522267d4","unresolved":false,"context_lines":[{"line_number":207,"context_line":"        LOG.error(_LE(\"The local_ip option must be set in either \""},{"line_number":208,"context_line":"                      \"openvswitch\u0027s configuration (under the [ovs] section) \""},{"line_number":209,"context_line":"                      \"and in linuxbridge\u0027s configuration (under the [vxlan] \""},{"line_number":210,"context_line":"                      \"section). if they are both present, they must match.\"))"},{"line_number":211,"context_line":"        return None"},{"line_number":212,"context_line":"    elif not _is_local_ip_valid():"},{"line_number":213,"context_line":"        return None"}],"source_content_type":"text/x-python","patch_set":14,"id":"9a80dd14_19ba5c0a","line":210,"updated":"2015-03-05 17:57:45.000000000","message":"why???","commit_id":"657be02549d3788066145de92e8da8eb8a4e6ce6"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"abac4cd7af019941e7ae95ac21520e89df4fcf61","unresolved":false,"context_lines":[{"line_number":207,"context_line":"        LOG.error(_LE(\"The local_ip option must be set in either \""},{"line_number":208,"context_line":"                      \"openvswitch\u0027s configuration (under the [ovs] section) \""},{"line_number":209,"context_line":"                      \"and in linuxbridge\u0027s configuration (under the [vxlan] \""},{"line_number":210,"context_line":"                      \"section). if they are both present, they must match.\"))"},{"line_number":211,"context_line":"        return None"},{"line_number":212,"context_line":"    elif not _is_local_ip_valid():"},{"line_number":213,"context_line":"        return None"}],"source_content_type":"text/x-python","patch_set":14,"id":"9a80dd14_c180618d","line":210,"in_reply_to":"9a80dd14_19ba5c0a","updated":"2015-03-09 14:23:03.000000000","message":"see above.","commit_id":"657be02549d3788066145de92e8da8eb8a4e6ce6"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"9d594936179850886294e1b7e8a1f46ee18d0002","unresolved":false,"context_lines":[{"line_number":34,"context_line":"# How many overhead bytes does each of the tunnel types force?"},{"line_number":35,"context_line":"TUNNEL_OVERHEAD \u003d {"},{"line_number":36,"context_line":"    \u0027gre\u0027: 42,"},{"line_number":37,"context_line":"    \u0027vxlan\u0027: 50,"},{"line_number":38,"context_line":"    \u0027stt\u0027: 72,"},{"line_number":39,"context_line":"}"},{"line_number":40,"context_line":""}],"source_content_type":"text/x-python","patch_set":16,"id":"da9b358b_167173e0","line":37,"updated":"2015-04-01 09:58:02.000000000","message":"+1","commit_id":"a27fad82f6acf55c53c06999dd2a2d97cb61ffab"},{"author":{"_account_id":6524,"name":"Henry Gessau","email":"HenryG@gessau.net","username":"gessau"},"change_message_id":"29e6285b6b3268e9df9d8e84aea641635a72360a","unresolved":false,"context_lines":[{"line_number":34,"context_line":"# How many overhead bytes does each of the tunnel types force?"},{"line_number":35,"context_line":"TUNNEL_OVERHEAD \u003d {"},{"line_number":36,"context_line":"    \u0027gre\u0027: 42,"},{"line_number":37,"context_line":"    \u0027vxlan\u0027: 50,"},{"line_number":38,"context_line":"    \u0027stt\u0027: 72,"},{"line_number":39,"context_line":"}"},{"line_number":40,"context_line":""}],"source_content_type":"text/x-python","patch_set":16,"id":"9a80dd14_5a12fd09","line":37,"updated":"2015-03-18 12:48:47.000000000","message":"The above two values are now in neutron/plugins/common/constants.py and you could add STT there.","commit_id":"a27fad82f6acf55c53c06999dd2a2d97cb61ffab"},{"author":{"_account_id":10370,"name":"Romil Gupta","email":"romilgupta19@gmail.com","username":"romilg"},"change_message_id":"a1bf4836a3562edf7e2ee8e8419e26fdfb9f86df","unresolved":false,"context_lines":[{"line_number":165,"context_line":"        return False"},{"line_number":166,"context_line":"    return True"},{"line_number":167,"context_line":""},{"line_number":168,"context_line":""},{"line_number":169,"context_line":"def _is_local_ip_valid():"},{"line_number":170,"context_line":"    \"\"\"True if both IPs can be successfully parsed and they match.\"\"\""},{"line_number":171,"context_line":""}],"source_content_type":"text/x-python","patch_set":16,"id":"9a80dd14_ef185a20","line":168,"updated":"2015-03-18 10:18:14.000000000","message":"How about making your patch-set dependent on this patch:\n\nhttps://review.openstack.org/#/c/154043/. \n\nAnd, remove the below method:\n    def _is_local_ip_valid():\n\nBy doing this, we could avoid redundancy as well as this check\n     LOG.error(_LE(\"Conflicting local_ip configuration found.\"))","commit_id":"a27fad82f6acf55c53c06999dd2a2d97cb61ffab"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"9d594936179850886294e1b7e8a1f46ee18d0002","unresolved":false,"context_lines":[{"line_number":165,"context_line":"        return False"},{"line_number":166,"context_line":"    return True"},{"line_number":167,"context_line":""},{"line_number":168,"context_line":""},{"line_number":169,"context_line":"def _is_local_ip_valid():"},{"line_number":170,"context_line":"    \"\"\"True if both IPs can be successfully parsed and they match.\"\"\""},{"line_number":171,"context_line":""}],"source_content_type":"text/x-python","patch_set":16,"id":"da9b358b_f6784fc6","line":168,"updated":"2015-04-01 09:58:02.000000000","message":"lets just wait and see which one merges first. dependent patches tend to slow things way down","commit_id":"a27fad82f6acf55c53c06999dd2a2d97cb61ffab"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"9d594936179850886294e1b7e8a1f46ee18d0002","unresolved":false,"context_lines":[{"line_number":176,"context_line":"                          if cfg.CONF.VXLAN.local_ip else None)"},{"line_number":177,"context_line":""},{"line_number":178,"context_line":"        if not ovs_local_ip or not vxlan_local_ip:"},{"line_number":179,"context_line":"            # We know from that at least one of them is not empty, so there"},{"line_number":180,"context_line":"            # can\u0027t be a conflict."},{"line_number":181,"context_line":"            return True"},{"line_number":182,"context_line":""}],"source_content_type":"text/x-python","patch_set":16,"id":"da9b358b_d6730be6","line":179,"updated":"2015-04-01 09:58:02.000000000","message":"This comment doesn\u0027t seem to match the logic. The logic is only confirming that 1 or both are None. They could both be empty...","commit_id":"a27fad82f6acf55c53c06999dd2a2d97cb61ffab"},{"author":{"_account_id":6524,"name":"Henry Gessau","email":"HenryG@gessau.net","username":"gessau"},"change_message_id":"29e6285b6b3268e9df9d8e84aea641635a72360a","unresolved":false,"context_lines":[{"line_number":241,"context_line":"                        \u0027at least %(needed_mtu)s for the configured tunnel \u0027"},{"line_number":242,"context_line":"                        \u0027types (the tunnel types currently configured are: \u0027"},{"line_number":243,"context_line":"                        \u0027%(tunnel_types)r and the maximum overhead is \u0027"},{"line_number":244,"context_line":"                        \u0027%(max_overhead)s)!\u0027),"},{"line_number":245,"context_line":"                    {\u0027local_ip\u0027: local_ip,"},{"line_number":246,"context_line":"                     \u0027current_mtu\u0027: current_mtu,"},{"line_number":247,"context_line":"                     \u0027needed_mtu\u0027: needed_mtu,"}],"source_content_type":"text/x-python","patch_set":16,"id":"9a80dd14_da48adb5","line":244,"updated":"2015-03-18 12:48:47.000000000","message":"With the bp/mtu-selection-and-advertisement now merged, this warning is missing some information. Operators also have the option of configuring advertise_mtu and the segment and path MTUs.","commit_id":"a27fad82f6acf55c53c06999dd2a2d97cb61ffab"}],"neutron/cmd/sanity_check.py":[{"author":{"_account_id":6524,"name":"Henry Gessau","email":"HenryG@gessau.net","username":"gessau"},"change_message_id":"59de2891000a63b2259f79a4565fd57cc4a6dc56","unresolved":false,"context_lines":[{"line_number":74,"context_line":"def check_mtu():"},{"line_number":75,"context_line":"    result \u003d checks.is_mtu_valid("},{"line_number":76,"context_line":"        root_helper\u003dcfg.CONF.AGENT.root_helper,"},{"line_number":77,"context_line":"        local_ip\u003dcfg.CONF.OVS.local_ip,"},{"line_number":78,"context_line":"        tunnel_types\u003dcfg.CONF.AGENT.tunnel_types)"},{"line_number":79,"context_line":"    if not result:"},{"line_number":80,"context_line":"        params \u003d checks.TUNNEL_OVERHEAD.copy()"}],"source_content_type":"text/x-python","patch_set":1,"id":"9aa7fdbe_6e80fd46","line":77,"updated":"2014-10-22 03:29:17.000000000","message":"Looking at the ini files, it seems that for the linuxbridge agent the local_ip option is in the [vxlan] section. Looks like you\u0027ll need to check both places.","commit_id":"5a8e1519a6805a310d6f573ee1b0ac31588ab90c"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"a89d711d24d06cc3a4cc1f2ab86e9073fd2f7381","unresolved":false,"context_lines":[{"line_number":74,"context_line":"def check_mtu():"},{"line_number":75,"context_line":"    result \u003d checks.is_mtu_valid("},{"line_number":76,"context_line":"        root_helper\u003dcfg.CONF.AGENT.root_helper,"},{"line_number":77,"context_line":"        local_ip\u003dcfg.CONF.OVS.local_ip,"},{"line_number":78,"context_line":"        tunnel_types\u003dcfg.CONF.AGENT.tunnel_types)"},{"line_number":79,"context_line":"    if not result:"},{"line_number":80,"context_line":"        params \u003d checks.TUNNEL_OVERHEAD.copy()"}],"source_content_type":"text/x-python","patch_set":1,"id":"9aa7fdbe_f7c83424","line":77,"in_reply_to":"9aa7fdbe_6e80fd46","updated":"2014-10-22 12:40:34.000000000","message":"I\u0027ll look into it before uploading a new patchset.","commit_id":"5a8e1519a6805a310d6f573ee1b0ac31588ab90c"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"1b69cf1d627169ec350e5dd9f242a3cb931483b0","unresolved":false,"context_lines":[{"line_number":74,"context_line":"def check_mtu():"},{"line_number":75,"context_line":"    result \u003d checks.is_mtu_valid("},{"line_number":76,"context_line":"        root_helper\u003dcfg.CONF.AGENT.root_helper,"},{"line_number":77,"context_line":"        local_ip\u003dcfg.CONF.OVS.local_ip,"},{"line_number":78,"context_line":"        tunnel_types\u003dcfg.CONF.AGENT.tunnel_types)"},{"line_number":79,"context_line":"    if not result:"},{"line_number":80,"context_line":"        params \u003d checks.TUNNEL_OVERHEAD.copy()"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a8c0949_6ca01a91","line":77,"in_reply_to":"9aa7fdbe_f7c83424","updated":"2014-11-03 11:56:45.000000000","message":"Done","commit_id":"5a8e1519a6805a310d6f573ee1b0ac31588ab90c"},{"author":{"_account_id":6524,"name":"Henry Gessau","email":"HenryG@gessau.net","username":"gessau"},"change_message_id":"59de2891000a63b2259f79a4565fd57cc4a6dc56","unresolved":false,"context_lines":[{"line_number":83,"context_line":"                        \u0027%(local_ip)s is not high enough and should be \u0027"},{"line_number":84,"context_line":"                        \u0027raised. Please note that the overhead for VXLAN is \u0027"},{"line_number":85,"context_line":"                        \u0027%(vxlan)s, the overhead for GRE is %(gre)s and the \u0027"},{"line_number":86,"context_line":"                        \u0027overhead for STT is %(stt)s.\u0027), params)"},{"line_number":87,"context_line":"    return result"},{"line_number":88,"context_line":""},{"line_number":89,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"9aa7fdbe_8ec44193","line":86,"updated":"2014-10-22 03:29:17.000000000","message":"I think it would be much nicer if we displayed to the user what MTU was found on the interface.\n\nDo the calculations for the user and then report:\n\n\"The interface for IP %(local_ip)s is %(mtu). This will allow guest MTU values of up to %(vxlan)s for VXLAN, %(gre)s for GRE, and %(stt)s for STT.\"\n\n(For extra karma points, report only for the tunnel types that are configured. :)","commit_id":"5a8e1519a6805a310d6f573ee1b0ac31588ab90c"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"a89d711d24d06cc3a4cc1f2ab86e9073fd2f7381","unresolved":false,"context_lines":[{"line_number":83,"context_line":"                        \u0027%(local_ip)s is not high enough and should be \u0027"},{"line_number":84,"context_line":"                        \u0027raised. Please note that the overhead for VXLAN is \u0027"},{"line_number":85,"context_line":"                        \u0027%(vxlan)s, the overhead for GRE is %(gre)s and the \u0027"},{"line_number":86,"context_line":"                        \u0027overhead for STT is %(stt)s.\u0027), params)"},{"line_number":87,"context_line":"    return result"},{"line_number":88,"context_line":""},{"line_number":89,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"9aa7fdbe_d7e1709b","line":86,"in_reply_to":"9aa7fdbe_8ec44193","updated":"2014-10-22 12:40:34.000000000","message":"You are right, though I intended to write this as closely to the other checks as possible. Probably a good idea to move this log to the other file, and then we can create a better log message.","commit_id":"5a8e1519a6805a310d6f573ee1b0ac31588ab90c"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"1b69cf1d627169ec350e5dd9f242a3cb931483b0","unresolved":false,"context_lines":[{"line_number":83,"context_line":"                        \u0027%(local_ip)s is not high enough and should be \u0027"},{"line_number":84,"context_line":"                        \u0027raised. Please note that the overhead for VXLAN is \u0027"},{"line_number":85,"context_line":"                        \u0027%(vxlan)s, the overhead for GRE is %(gre)s and the \u0027"},{"line_number":86,"context_line":"                        \u0027overhead for STT is %(stt)s.\u0027), params)"},{"line_number":87,"context_line":"    return result"},{"line_number":88,"context_line":""},{"line_number":89,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"7a8c0949_ac1da2bf","line":86,"in_reply_to":"9aa7fdbe_d7e1709b","updated":"2014-11-03 11:56:45.000000000","message":"I changed the text to provide more info (the needed mtu to eliminate problems). Is this better?","commit_id":"5a8e1519a6805a310d6f573ee1b0ac31588ab90c"},{"author":{"_account_id":6524,"name":"Henry Gessau","email":"HenryG@gessau.net","username":"gessau"},"change_message_id":"59de2891000a63b2259f79a4565fd57cc4a6dc56","unresolved":false,"context_lines":[{"line_number":97,"context_line":"                    help\u003d_(\u0027Check for nova notification support\u0027)),"},{"line_number":98,"context_line":"    BoolOptCallback(\u0027arp_responder\u0027, check_arp_responder, default\u003dFalse,"},{"line_number":99,"context_line":"                    help\u003d_(\u0027Check for ARP responder support\u0027)),"},{"line_number":100,"context_line":"    BoolOptCallback(\u0027check_mtu\u0027, check_mtu, default\u003dFalse,"},{"line_number":101,"context_line":"                    help\u003d_(\u0027Check for MTU inconsistencies\u0027)),"},{"line_number":102,"context_line":"]"},{"line_number":103,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"9aa7fdbe_2e6275ad","line":100,"updated":"2014-10-22 03:29:17.000000000","message":"I hate that all these options use underscore instead of dash. :(","commit_id":"5a8e1519a6805a310d6f573ee1b0ac31588ab90c"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"a89d711d24d06cc3a4cc1f2ab86e9073fd2f7381","unresolved":false,"context_lines":[{"line_number":97,"context_line":"                    help\u003d_(\u0027Check for nova notification support\u0027)),"},{"line_number":98,"context_line":"    BoolOptCallback(\u0027arp_responder\u0027, check_arp_responder, default\u003dFalse,"},{"line_number":99,"context_line":"                    help\u003d_(\u0027Check for ARP responder support\u0027)),"},{"line_number":100,"context_line":"    BoolOptCallback(\u0027check_mtu\u0027, check_mtu, default\u003dFalse,"},{"line_number":101,"context_line":"                    help\u003d_(\u0027Check for MTU inconsistencies\u0027)),"},{"line_number":102,"context_line":"]"},{"line_number":103,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"9aa7fdbe_57ed808a","line":100,"in_reply_to":"9aa7fdbe_2e6275ad","updated":"2014-10-22 12:40:34.000000000","message":"Likewise, though it\u0027s out of the scope of my patch :(","commit_id":"5a8e1519a6805a310d6f573ee1b0ac31588ab90c"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"a89d711d24d06cc3a4cc1f2ab86e9073fd2f7381","unresolved":false,"context_lines":[{"line_number":139,"context_line":""},{"line_number":140,"context_line":""},{"line_number":141,"context_line":"if __name__ \u003d\u003d \u0027__main__\u0027:"},{"line_number":142,"context_line":"    main()"}],"source_content_type":"text/x-python","patch_set":1,"id":"9aa7fdbe_b7fdcc3c","line":142,"updated":"2014-10-22 12:40:34.000000000","message":"This should be removed in the next patchset.","commit_id":"5a8e1519a6805a310d6f573ee1b0ac31588ab90c"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"1b69cf1d627169ec350e5dd9f242a3cb931483b0","unresolved":false,"context_lines":[{"line_number":139,"context_line":""},{"line_number":140,"context_line":""},{"line_number":141,"context_line":"if __name__ \u003d\u003d \u0027__main__\u0027:"},{"line_number":142,"context_line":"    main()"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a8c0949_0c5b1688","line":142,"in_reply_to":"9aa7fdbe_b7fdcc3c","updated":"2014-11-03 11:56:45.000000000","message":"Done.","commit_id":"5a8e1519a6805a310d6f573ee1b0ac31588ab90c"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"055fddfe35ad998250d6cc53564f6f99f77282b9","unresolved":false,"context_lines":[{"line_number":91,"context_line":"    if (not local_ip or"},{"line_number":92,"context_line":"        (cfg.CONF.OVS.local_ip and cfg.CONF.VXLAN.local_ip and"},{"line_number":93,"context_line":"         cfg.CONF.OVS.local_ip !\u003d cfg.CONF.VXLAN.local_ip)):"},{"line_number":94,"context_line":"        LOG.error(_LE(\u0027Could not determine local_ip. Please make sure that \u0027"},{"line_number":95,"context_line":"                      \u0027both configuration options (in [ovs] and [vxlan] \u0027"},{"line_number":96,"context_line":"                      \u0027match to each other and the actual IP address this \u0027"},{"line_number":97,"context_line":"                      \u0027node uses.\u0027))"}],"source_content_type":"text/x-python","patch_set":2,"id":"5a890539_72c71746","line":94,"updated":"2014-11-11 23:24:57.000000000","message":"Error message doesn\u0027t address the \"not local_ip\" condition. Would it be clearer to have two error messages, one for no IP set, and one for mismatched IP?","commit_id":"7ab07e80dad964281a59c8e2465f6214c8483bbe"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"8a4ef221a94dc7eb8d59b05bc2604f7504516ec5","unresolved":false,"context_lines":[{"line_number":91,"context_line":"    if (not local_ip or"},{"line_number":92,"context_line":"        (cfg.CONF.OVS.local_ip and cfg.CONF.VXLAN.local_ip and"},{"line_number":93,"context_line":"         cfg.CONF.OVS.local_ip !\u003d cfg.CONF.VXLAN.local_ip)):"},{"line_number":94,"context_line":"        LOG.error(_LE(\u0027Could not determine local_ip. Please make sure that \u0027"},{"line_number":95,"context_line":"                      \u0027both configuration options (in [ovs] and [vxlan] \u0027"},{"line_number":96,"context_line":"                      \u0027match to each other and the actual IP address this \u0027"},{"line_number":97,"context_line":"                      \u0027node uses.\u0027))"}],"source_content_type":"text/x-python","patch_set":2,"id":"5a890539_d1135f0b","line":94,"in_reply_to":"5a890539_72c71746","updated":"2014-11-17 07:15:43.000000000","message":"Done","commit_id":"7ab07e80dad964281a59c8e2465f6214c8483bbe"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"055fddfe35ad998250d6cc53564f6f99f77282b9","unresolved":false,"context_lines":[{"line_number":92,"context_line":"        (cfg.CONF.OVS.local_ip and cfg.CONF.VXLAN.local_ip and"},{"line_number":93,"context_line":"         cfg.CONF.OVS.local_ip !\u003d cfg.CONF.VXLAN.local_ip)):"},{"line_number":94,"context_line":"        LOG.error(_LE(\u0027Could not determine local_ip. Please make sure that \u0027"},{"line_number":95,"context_line":"                      \u0027both configuration options (in [ovs] and [vxlan] \u0027"},{"line_number":96,"context_line":"                      \u0027match to each other and the actual IP address this \u0027"},{"line_number":97,"context_line":"                      \u0027node uses.\u0027))"},{"line_number":98,"context_line":"        return False"}],"source_content_type":"text/x-python","patch_set":2,"id":"5a890539_129a5363","line":95,"updated":"2014-11-11 23:24:57.000000000","message":"[vxlan] -\u003e [vxlan])","commit_id":"7ab07e80dad964281a59c8e2465f6214c8483bbe"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"8a4ef221a94dc7eb8d59b05bc2604f7504516ec5","unresolved":false,"context_lines":[{"line_number":92,"context_line":"        (cfg.CONF.OVS.local_ip and cfg.CONF.VXLAN.local_ip and"},{"line_number":93,"context_line":"         cfg.CONF.OVS.local_ip !\u003d cfg.CONF.VXLAN.local_ip)):"},{"line_number":94,"context_line":"        LOG.error(_LE(\u0027Could not determine local_ip. Please make sure that \u0027"},{"line_number":95,"context_line":"                      \u0027both configuration options (in [ovs] and [vxlan] \u0027"},{"line_number":96,"context_line":"                      \u0027match to each other and the actual IP address this \u0027"},{"line_number":97,"context_line":"                      \u0027node uses.\u0027))"},{"line_number":98,"context_line":"        return False"}],"source_content_type":"text/x-python","patch_set":2,"id":"5a890539_3104a3bf","line":95,"in_reply_to":"5a890539_129a5363","updated":"2014-11-17 07:15:43.000000000","message":"Done","commit_id":"7ab07e80dad964281a59c8e2465f6214c8483bbe"},{"author":{"_account_id":6598,"name":"Berezovsky Irena","email":"irenab.dev@gmail.com","username":"irenab"},"change_message_id":"e1dad24452aa110e4ed93ca008aee16e52df5933","unresolved":false,"context_lines":[{"line_number":103,"context_line":"                      \"actual IP address this node uses.\"))"},{"line_number":104,"context_line":"        return False"},{"line_number":105,"context_line":""},{"line_number":106,"context_line":"    result \u003d checks.is_mtu_valid("},{"line_number":107,"context_line":"        local_ip\u003dlocal_ip,"},{"line_number":108,"context_line":"        tunnel_types\u003dcfg.CONF.AGENT.tunnel_types)"},{"line_number":109,"context_line":"    return result"}],"source_content_type":"text/x-python","patch_set":4,"id":"5a890539_4acd77b6","line":106,"updated":"2014-11-17 09:28:04.000000000","message":"till this section, code seems not related to check_mtu. Please consider separate checking","commit_id":"5d9a54b79727d472ab20f3a4a0e058c46b24f8bd"},{"author":{"_account_id":6598,"name":"Berezovsky Irena","email":"irenab.dev@gmail.com","username":"irenab"},"change_message_id":"a6c8f26f52a756f300c1724eb17fd2fa9dfd8dcb","unresolved":false,"context_lines":[{"line_number":103,"context_line":"                      \"actual IP address this node uses.\"))"},{"line_number":104,"context_line":"        return False"},{"line_number":105,"context_line":""},{"line_number":106,"context_line":"    result \u003d checks.is_mtu_valid("},{"line_number":107,"context_line":"        local_ip\u003dlocal_ip,"},{"line_number":108,"context_line":"        tunnel_types\u003dcfg.CONF.AGENT.tunnel_types)"},{"line_number":109,"context_line":"    return result"}],"source_content_type":"text/x-python","patch_set":4,"id":"5a890539_2d7be08c","line":106,"in_reply_to":"5a890539_17ce372e","updated":"2014-11-17 12:46:56.000000000","message":"Thank you for addressing my comment.","commit_id":"5d9a54b79727d472ab20f3a4a0e058c46b24f8bd"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"e724e7c9af14a279601b31593d6f5689b86f1b2a","unresolved":false,"context_lines":[{"line_number":103,"context_line":"                      \"actual IP address this node uses.\"))"},{"line_number":104,"context_line":"        return False"},{"line_number":105,"context_line":""},{"line_number":106,"context_line":"    result \u003d checks.is_mtu_valid("},{"line_number":107,"context_line":"        local_ip\u003dlocal_ip,"},{"line_number":108,"context_line":"        tunnel_types\u003dcfg.CONF.AGENT.tunnel_types)"},{"line_number":109,"context_line":"    return result"}],"source_content_type":"text/x-python","patch_set":4,"id":"5a890539_17ce372e","line":106,"in_reply_to":"5a890539_4acd77b6","updated":"2014-11-17 11:57:28.000000000","message":"Irena,\n\nWhile I understand your concern, splitting this to two different checks is quite a mess because \u0027check_mtu\u0027 will depend on \u0027check_local_ip\u0027 (we wouldn\u0027t want to even check the mtu if the local_ip couldn\u0027t be determined) and there\u0027s no way to \u0027link\u0027 2 checks together (ie. force running check_local_ip exactly once if check_mtu is used) without doing some heavy refactoring which is out of scope for this patch.\n\nI settled on splitting the 2 logics to 2 different functions - the local_ip retrieving and checking is done in a private function called directly by check_mtu. Please review the next patchset :)","commit_id":"5d9a54b79727d472ab20f3a4a0e058c46b24f8bd"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"e59fdd5f0b644cf1f1859955f9d862fbf1954d27","unresolved":false,"context_lines":[{"line_number":92,"context_line":"    \"\"\""},{"line_number":93,"context_line":"    local_ip \u003d cfg.CONF.OVS.local_ip or cfg.CONF.VXLAN.local_ip"},{"line_number":94,"context_line":"    if not local_ip:"},{"line_number":95,"context_line":"        LOG.error(_LE(\"the local_ip option must be set in either \""},{"line_number":96,"context_line":"                      \"openvswitch\u0027s configuration (under the [ovs] section) \""},{"line_number":97,"context_line":"                      \"and in linuxbridge\u0027s configuration (under the [vxlan] \""},{"line_number":98,"context_line":"                      \"section). if they are both present, they must match.\"))"}],"source_content_type":"text/x-python","patch_set":5,"id":"5a890539_743e2c2b","line":95,"updated":"2014-11-19 23:18:53.000000000","message":"Nit: the -\u003e The","commit_id":"8e4297b3c96b071a3e251833fb5125fd957d4f7e"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"7c85eca6ae4bdd495e6212b1691cff64c7630b31","unresolved":false,"context_lines":[{"line_number":92,"context_line":"    \"\"\""},{"line_number":93,"context_line":"    local_ip \u003d cfg.CONF.OVS.local_ip or cfg.CONF.VXLAN.local_ip"},{"line_number":94,"context_line":"    if not local_ip:"},{"line_number":95,"context_line":"        LOG.error(_LE(\"the local_ip option must be set in either \""},{"line_number":96,"context_line":"                      \"openvswitch\u0027s configuration (under the [ovs] section) \""},{"line_number":97,"context_line":"                      \"and in linuxbridge\u0027s configuration (under the [vxlan] \""},{"line_number":98,"context_line":"                      \"section). if they are both present, they must match.\"))"}],"source_content_type":"text/x-python","patch_set":5,"id":"3a961159_41987d75","line":95,"in_reply_to":"5a890539_743e2c2b","updated":"2015-01-13 07:59:26.000000000","message":"Done","commit_id":"8e4297b3c96b071a3e251833fb5125fd957d4f7e"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"e59fdd5f0b644cf1f1859955f9d862fbf1954d27","unresolved":false,"context_lines":[{"line_number":108,"context_line":"    return local_ip"},{"line_number":109,"context_line":""},{"line_number":110,"context_line":""},{"line_number":111,"context_line":"def check_mtu():"},{"line_number":112,"context_line":"    local_ip \u003d _get_local_ip()"},{"line_number":113,"context_line":"    if not local_ip:"},{"line_number":114,"context_line":"        return False"}],"source_content_type":"text/x-python","patch_set":5,"id":"5a890539_482b5030","line":111,"updated":"2014-11-19 23:18:53.000000000","message":"Suggestion for rearranging this check so that your UTs don\u0027t have to check log messages...\n\nYou could do something like this:\n\n    def _local_ip_not_configured(local_ip):\n        if not local_ip:\n            LOG.error(_LE(\"The local_ip option...\"))\n            return True\n\n    def _conflicting_local_ip():\n        if (cfg.CONF.OVS.local_ip and ...):\n            LOG.error(_LE(\"Conflicting local_ip...\"))\n            return True\n\n    def check_mtu():\n        local_ip \u003d cfg.CONF.OVS.local_ip or cfg.CONF.VXLAN.local_ip\n        if _local_ip_not_configured():\n            return False\n        if _conflicing_local_ip():\n            return False\n        ...\n\nNow UT can directly test the private methods and one test can check happy path by testing check_mtu().","commit_id":"8e4297b3c96b071a3e251833fb5125fd957d4f7e"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"7c85eca6ae4bdd495e6212b1691cff64c7630b31","unresolved":false,"context_lines":[{"line_number":108,"context_line":"    return local_ip"},{"line_number":109,"context_line":""},{"line_number":110,"context_line":""},{"line_number":111,"context_line":"def check_mtu():"},{"line_number":112,"context_line":"    local_ip \u003d _get_local_ip()"},{"line_number":113,"context_line":"    if not local_ip:"},{"line_number":114,"context_line":"        return False"}],"source_content_type":"text/x-python","patch_set":5,"id":"3a961159_21745135","line":111,"in_reply_to":"5a890539_482b5030","updated":"2015-01-13 07:59:26.000000000","message":"Thanks for the idea! Implemented :)","commit_id":"8e4297b3c96b071a3e251833fb5125fd957d4f7e"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"bdadc1afcc8cee10866ca252745ae4efc0db5527","unresolved":false,"context_lines":[{"line_number":116,"context_line":"                      \u0027being used has VF support.\u0027))"},{"line_number":117,"context_line":""},{"line_number":118,"context_line":""},{"line_number":119,"context_line":"def _is_local_ip_configured(local_ip):"},{"line_number":120,"context_line":"    if not local_ip:"},{"line_number":121,"context_line":"        LOG.error(_LE(\"The local_ip option must be set in either \""},{"line_number":122,"context_line":"                      \"openvswitch\u0027s configuration (under the [ovs] section) \""}],"source_content_type":"text/x-python","patch_set":7,"id":"3a961159_a3c2e531","line":119,"updated":"2015-01-14 00:36:25.000000000","message":"This seems more like a \"_warn_if_no_local_ip\"\n\nOtherwise, it is nothing more than \"bool(local_ip)\"","commit_id":"a0c572b7a57638424caae7a95d2a6e95231f60ae"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"f03673f0c4fbdaba5ae24e0a1bbd821e97b364f9","unresolved":false,"context_lines":[{"line_number":116,"context_line":"                      \u0027being used has VF support.\u0027))"},{"line_number":117,"context_line":""},{"line_number":118,"context_line":""},{"line_number":119,"context_line":"def _is_local_ip_configured(local_ip):"},{"line_number":120,"context_line":"    if not local_ip:"},{"line_number":121,"context_line":"        LOG.error(_LE(\"The local_ip option must be set in either \""},{"line_number":122,"context_line":"                      \"openvswitch\u0027s configuration (under the [ovs] section) \""}],"source_content_type":"text/x-python","patch_set":7,"id":"3a961159_afc55f0b","line":119,"in_reply_to":"3a961159_09eaf373","updated":"2015-01-14 10:31:33.000000000","message":"\u0027_warn_if..\u0027 seems a bit off as it doesn\u0027t imply strongly enough that the function checks anything.\n\nThe reason for these 2 functions can be found in PS#5 - basically for the unit test to work reliably without mocking the logging module, I split this out so I can check the actual return value for these conditions. Also, the other file doesn\u0027t have an oslo.config import and I was reluctant to add it...\n\nAnyway, I\u0027ve trimmed this down as suggested, and the unit tests will be a bit more simplistic (will still check this case).","commit_id":"a0c572b7a57638424caae7a95d2a6e95231f60ae"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"e38876b52a2d6d571a57377932c9d1cd8c8a8d05","unresolved":false,"context_lines":[{"line_number":116,"context_line":"                      \u0027being used has VF support.\u0027))"},{"line_number":117,"context_line":""},{"line_number":118,"context_line":""},{"line_number":119,"context_line":"def _is_local_ip_configured(local_ip):"},{"line_number":120,"context_line":"    if not local_ip:"},{"line_number":121,"context_line":"        LOG.error(_LE(\"The local_ip option must be set in either \""},{"line_number":122,"context_line":"                      \"openvswitch\u0027s configuration (under the [ovs] section) \""}],"source_content_type":"text/x-python","patch_set":7,"id":"3a961159_09eaf373","line":119,"in_reply_to":"3a961159_a3c2e531","updated":"2015-01-14 07:56:13.000000000","message":"+1","commit_id":"a0c572b7a57638424caae7a95d2a6e95231f60ae"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"bdadc1afcc8cee10866ca252745ae4efc0db5527","unresolved":false,"context_lines":[{"line_number":128,"context_line":""},{"line_number":129,"context_line":"def _is_local_ip_conflicting():"},{"line_number":130,"context_line":"    if (cfg.CONF.OVS.local_ip and cfg.CONF.VXLAN.local_ip and"},{"line_number":131,"context_line":"        cfg.CONF.OVS.local_ip !\u003d cfg.CONF.VXLAN.local_ip):"},{"line_number":132,"context_line":"        LOG.error(_LE(\"Conflicting local_ip configuration found. Please make \""},{"line_number":133,"context_line":"                      \"sure that both openvswitch\u0027s and linuxbridge\u0027s \""},{"line_number":134,"context_line":"                      \"configuration options (in the [ovs] and [vxlan] \""}],"source_content_type":"text/x-python","patch_set":7,"id":"3a961159_9e60ded0","line":131,"updated":"2015-01-14 00:36:25.000000000","message":"This simplistic comparison may not work for ipv6 addresses which should compare the same but may not be written the same.  Using netaddr should fix that.  For example:\n\n\u003e\u003e\u003e netaddr.IPAddress(\u0027::1\u0027) \u003d\u003d netaddr.IPAddress(\u00270000:0000:0000:0000:0000:0000:0000:0001\u0027)\nTrue","commit_id":"a0c572b7a57638424caae7a95d2a6e95231f60ae"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"f03673f0c4fbdaba5ae24e0a1bbd821e97b364f9","unresolved":false,"context_lines":[{"line_number":128,"context_line":""},{"line_number":129,"context_line":"def _is_local_ip_conflicting():"},{"line_number":130,"context_line":"    if (cfg.CONF.OVS.local_ip and cfg.CONF.VXLAN.local_ip and"},{"line_number":131,"context_line":"        cfg.CONF.OVS.local_ip !\u003d cfg.CONF.VXLAN.local_ip):"},{"line_number":132,"context_line":"        LOG.error(_LE(\"Conflicting local_ip configuration found. Please make \""},{"line_number":133,"context_line":"                      \"sure that both openvswitch\u0027s and linuxbridge\u0027s \""},{"line_number":134,"context_line":"                      \"configuration options (in the [ovs] and [vxlan] \""}],"source_content_type":"text/x-python","patch_set":7,"id":"3a961159_efcf6727","line":131,"in_reply_to":"3a961159_9e60ded0","updated":"2015-01-14 10:31:33.000000000","message":"Good idea. Done.","commit_id":"a0c572b7a57638424caae7a95d2a6e95231f60ae"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"e38876b52a2d6d571a57377932c9d1cd8c8a8d05","unresolved":false,"context_lines":[{"line_number":133,"context_line":"                      \"sure that both openvswitch\u0027s and linuxbridge\u0027s \""},{"line_number":134,"context_line":"                      \"configuration options (in the [ovs] and [vxlan] \""},{"line_number":135,"context_line":"                      \"sections, respectively) match to each other and the \""},{"line_number":136,"context_line":"                      \"actual IP address this node uses.\"))"},{"line_number":137,"context_line":"        return True"},{"line_number":138,"context_line":"    return False"},{"line_number":139,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"3a961159_69fa9744","line":136,"updated":"2015-01-14 07:56:13.000000000","message":"I would separate the LOG.error and the condition concerns","commit_id":"a0c572b7a57638424caae7a95d2a6e95231f60ae"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"d3265cfb1a5186a78b92c3650c853311b8fc9783","unresolved":false,"context_lines":[{"line_number":133,"context_line":"                      \"sure that both openvswitch\u0027s and linuxbridge\u0027s \""},{"line_number":134,"context_line":"                      \"configuration options (in the [ovs] and [vxlan] \""},{"line_number":135,"context_line":"                      \"sections, respectively) match to each other and the \""},{"line_number":136,"context_line":"                      \"actual IP address this node uses.\"))"},{"line_number":137,"context_line":"        return True"},{"line_number":138,"context_line":"    return False"},{"line_number":139,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"3a961159_e16cdbd2","line":136,"in_reply_to":"3a961159_69fa9744","updated":"2015-01-14 09:23:57.000000000","message":"I mean, there are two concerns on this function:\n\n1) Returning if \"_is_local_ip_conflicting\"\n2) Logging error.\n\nI would move the LOG.error to L152 at least, but then\n\n_get_local_ip doesn\u0027t imply we\u0027re \"logging errors\",  we may have a separate function for logging error conditions \n\n\nWhat about _check_local_ip()  and _get_local_ip() ? \n\nif _check_local_ip():\n     local_ip \u003d _get_local_ip()\n\n\nRecently I finding very cryptic (or less idiomatic) when I find functions that say to do an specific thing, and do a few other things... but I understand logging could be a corner case.","commit_id":"a0c572b7a57638424caae7a95d2a6e95231f60ae"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"f03673f0c4fbdaba5ae24e0a1bbd821e97b364f9","unresolved":false,"context_lines":[{"line_number":133,"context_line":"                      \"sure that both openvswitch\u0027s and linuxbridge\u0027s \""},{"line_number":134,"context_line":"                      \"configuration options (in the [ovs] and [vxlan] \""},{"line_number":135,"context_line":"                      \"sections, respectively) match to each other and the \""},{"line_number":136,"context_line":"                      \"actual IP address this node uses.\"))"},{"line_number":137,"context_line":"        return True"},{"line_number":138,"context_line":"    return False"},{"line_number":139,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"3a961159_6118eb1e","line":136,"in_reply_to":"3a961159_e16cdbd2","updated":"2015-01-14 10:31:33.000000000","message":"After discussing this with Miguel, I changed the code to match his idea.","commit_id":"a0c572b7a57638424caae7a95d2a6e95231f60ae"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"bdadc1afcc8cee10866ca252745ae4efc0db5527","unresolved":false,"context_lines":[{"line_number":138,"context_line":"    return False"},{"line_number":139,"context_line":""},{"line_number":140,"context_line":""},{"line_number":141,"context_line":"def _get_local_ip():"},{"line_number":142,"context_line":"    \"\"\"Retrieve the \u0027local_ip\u0027 option from the config and make sure it\u0027s valid."},{"line_number":143,"context_line":""},{"line_number":144,"context_line":"    \u0027local_ip\u0027 is configured in the OVS (for openvswitch) and in the VXLAN (for"}],"source_content_type":"text/x-python","patch_set":7,"id":"3a961159_039739ec","line":141,"updated":"2015-01-14 00:36:25.000000000","message":"Seems like this and the previous two methods should be moved to sanity/checks.py and called from is_mtu_valid.  The choice of what to implement here in this file and what to implement there seems arbitrary in this patch.  However, the precedent for this patch is to define a single check_ blah_blah method in this file which call a method in checks.py.","commit_id":"a0c572b7a57638424caae7a95d2a6e95231f60ae"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"f03673f0c4fbdaba5ae24e0a1bbd821e97b364f9","unresolved":false,"context_lines":[{"line_number":138,"context_line":"    return False"},{"line_number":139,"context_line":""},{"line_number":140,"context_line":""},{"line_number":141,"context_line":"def _get_local_ip():"},{"line_number":142,"context_line":"    \"\"\"Retrieve the \u0027local_ip\u0027 option from the config and make sure it\u0027s valid."},{"line_number":143,"context_line":""},{"line_number":144,"context_line":"    \u0027local_ip\u0027 is configured in the OVS (for openvswitch) and in the VXLAN (for"}],"source_content_type":"text/x-python","patch_set":7,"id":"3a961159_2f240f0d","line":141,"in_reply_to":"3a961159_039739ec","updated":"2015-01-14 10:31:33.000000000","message":"When I first wrote the code, the convention seemed to be that any logging should be done in this file - so these functions which produced log errors were forced to be left here. Since this is no longer the case, I moved these as suggested.","commit_id":"a0c572b7a57638424caae7a95d2a6e95231f60ae"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"31d87465d5a42923ab760a6367b444f4298a04ad","unresolved":false,"context_lines":[{"line_number":147,"context_line":"    BoolOptCallback(\u0027read_netns\u0027, check_read_netns,"},{"line_number":148,"context_line":"                    help\u003d_(\u0027Check netns permission settings\u0027)),"},{"line_number":149,"context_line":"    BoolOptCallback(\u0027check_mtu\u0027, check_mtu, default\u003dFalse,"},{"line_number":150,"context_line":"                    help\u003d_(\u0027Check for MTU inconsistencies\u0027)),"},{"line_number":151,"context_line":"]"},{"line_number":152,"context_line":""},{"line_number":153,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"3a961159_b1edb290","line":150,"updated":"2015-01-14 16:41:20.000000000","message":"Nit: period at end of help string message. Ref: http://docs.openstack.org/developer/oslo.config/styleguide.html\n\n(I imagine you were just being consistent with other help messages in file).","commit_id":"8d2beac509aecf04b28d499799b4abc4bdde8d42"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"bbff3817a85e7f2211bd9013635f698f34938079","unresolved":false,"context_lines":[{"line_number":124,"context_line":"    if not local_ip:"},{"line_number":125,"context_line":"        return False"},{"line_number":126,"context_line":""},{"line_number":127,"context_line":"    # The local_ip configuration is valid - check if its MTU is also valid."},{"line_number":128,"context_line":"    return checks.is_mtu_valid("},{"line_number":129,"context_line":"        local_ip\u003dlocal_ip,"},{"line_number":130,"context_line":"        tunnel_types\u003dcfg.CONF.AGENT.tunnel_types)"}],"source_content_type":"text/x-python","patch_set":9,"id":"3a961159_056eb397","line":127,"updated":"2015-01-19 22:27:45.000000000","message":"i would prefer to remove this comment and add one before L125","commit_id":"862edba89d598d97ef3ba6d2aa66d1d372c2de8f"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"dd5532bde674c6db1877d1a179e9bd3504d326aa","unresolved":false,"context_lines":[{"line_number":124,"context_line":"    if not local_ip:"},{"line_number":125,"context_line":"        return False"},{"line_number":126,"context_line":""},{"line_number":127,"context_line":"    # The local_ip configuration is valid - check if its MTU is also valid."},{"line_number":128,"context_line":"    return checks.is_mtu_valid("},{"line_number":129,"context_line":"        local_ip\u003dlocal_ip,"},{"line_number":130,"context_line":"        tunnel_types\u003dcfg.CONF.AGENT.tunnel_types)"}],"source_content_type":"text/x-python","patch_set":9,"id":"3a961159_13435214","line":127,"in_reply_to":"3a961159_056eb397","updated":"2015-01-20 07:49:32.000000000","message":"Done","commit_id":"862edba89d598d97ef3ba6d2aa66d1d372c2de8f"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"46e0fe8b802013488337da351396b1b4ea338390","unresolved":false,"context_lines":[{"line_number":114,"context_line":"        LOG.error(_LE(\u0027Check for VF management support failed. \u0027"},{"line_number":115,"context_line":"                      \u0027Please ensure that the version of ip link \u0027"},{"line_number":116,"context_line":"                      \u0027being used has VF support.\u0027))"},{"line_number":117,"context_line":""},{"line_number":118,"context_line":""},{"line_number":119,"context_line":"def check_mtu():"},{"line_number":120,"context_line":"    local_ip \u003d checks.get_local_ip("}],"source_content_type":"text/x-python","patch_set":10,"id":"1a930d6b_8ed01bf9","line":117,"updated":"2015-01-20 21:50:50.000000000","message":"It looks like you dropped the return value from check_vf_management() on accident.","commit_id":"537d72c3d20571f0552927675b5c8cc8604e3d19"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"669ebfdd3bd7bbdf2daec7ce45e763903689c0b6","unresolved":false,"context_lines":[{"line_number":114,"context_line":"        LOG.error(_LE(\u0027Check for VF management support failed. \u0027"},{"line_number":115,"context_line":"                      \u0027Please ensure that the version of ip link \u0027"},{"line_number":116,"context_line":"                      \u0027being used has VF support.\u0027))"},{"line_number":117,"context_line":""},{"line_number":118,"context_line":""},{"line_number":119,"context_line":"def check_mtu():"},{"line_number":120,"context_line":"    local_ip \u003d checks.get_local_ip("}],"source_content_type":"text/x-python","patch_set":10,"id":"1a930d6b_5d8fb923","line":117,"in_reply_to":"1a930d6b_8ed01bf9","updated":"2015-01-21 09:39:13.000000000","message":"Whoops. Must have been not careful enough in the last rebase.","commit_id":"537d72c3d20571f0552927675b5c8cc8604e3d19"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"5b4d1d9431f1e70d68a0a254c91baf8c09dd3a4c","unresolved":false,"context_lines":[{"line_number":146,"context_line":"                    help\u003d_(\u0027Check for VF management support\u0027)),"},{"line_number":147,"context_line":"    BoolOptCallback(\u0027read_netns\u0027, check_read_netns,"},{"line_number":148,"context_line":"                    help\u003d_(\u0027Check netns permission settings\u0027)),"},{"line_number":149,"context_line":"    BoolOptCallback(\u0027check_mtu\u0027, check_mtu, default\u003dFalse,"},{"line_number":150,"context_line":"                    help\u003d_(\u0027Check for MTU inconsistencies\u0027)),"},{"line_number":151,"context_line":"]"},{"line_number":152,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"3a961159_98a9047f","line":149,"updated":"2015-01-20 19:44:02.000000000","message":"See my style guide comment for help strings in PS 8.","commit_id":"537d72c3d20571f0552927675b5c8cc8604e3d19"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"fe18eea60b10a8e1e462886485874dd71445fcc3","unresolved":false,"context_lines":[{"line_number":146,"context_line":"                    help\u003d_(\u0027Check for VF management support\u0027)),"},{"line_number":147,"context_line":"    BoolOptCallback(\u0027read_netns\u0027, check_read_netns,"},{"line_number":148,"context_line":"                    help\u003d_(\u0027Check netns permission settings\u0027)),"},{"line_number":149,"context_line":"    BoolOptCallback(\u0027check_mtu\u0027, check_mtu, default\u003dFalse,"},{"line_number":150,"context_line":"                    help\u003d_(\u0027Check for MTU inconsistencies\u0027)),"},{"line_number":151,"context_line":"]"},{"line_number":152,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"1a930d6b_6c766e40","line":149,"in_reply_to":"1a930d6b_70372fb1","updated":"2015-01-21 21:26:21.000000000","message":"I could, but that\u0027s out of the scope of this patch... I\u0027d add it but then I\u0027d be asked to remove it for \u0027unrelated change\u0027 reasons.","commit_id":"537d72c3d20571f0552927675b5c8cc8604e3d19"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"d1218220270b961df170d215d0566216f556ac82","unresolved":false,"context_lines":[{"line_number":146,"context_line":"                    help\u003d_(\u0027Check for VF management support\u0027)),"},{"line_number":147,"context_line":"    BoolOptCallback(\u0027read_netns\u0027, check_read_netns,"},{"line_number":148,"context_line":"                    help\u003d_(\u0027Check netns permission settings\u0027)),"},{"line_number":149,"context_line":"    BoolOptCallback(\u0027check_mtu\u0027, check_mtu, default\u003dFalse,"},{"line_number":150,"context_line":"                    help\u003d_(\u0027Check for MTU inconsistencies\u0027)),"},{"line_number":151,"context_line":"]"},{"line_number":152,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"1a930d6b_70372fb1","line":149,"in_reply_to":"1a930d6b_dd5d694a","updated":"2015-01-21 15:13:42.000000000","message":"You can always fix all of them to be consistent...","commit_id":"537d72c3d20571f0552927675b5c8cc8604e3d19"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"669ebfdd3bd7bbdf2daec7ce45e763903689c0b6","unresolved":false,"context_lines":[{"line_number":146,"context_line":"                    help\u003d_(\u0027Check for VF management support\u0027)),"},{"line_number":147,"context_line":"    BoolOptCallback(\u0027read_netns\u0027, check_read_netns,"},{"line_number":148,"context_line":"                    help\u003d_(\u0027Check netns permission settings\u0027)),"},{"line_number":149,"context_line":"    BoolOptCallback(\u0027check_mtu\u0027, check_mtu, default\u003dFalse,"},{"line_number":150,"context_line":"                    help\u003d_(\u0027Check for MTU inconsistencies\u0027)),"},{"line_number":151,"context_line":"]"},{"line_number":152,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"1a930d6b_dd5d694a","line":149,"in_reply_to":"3a961159_98a9047f","updated":"2015-01-21 09:39:13.000000000","message":"Whoops, I completely missed that.\n\nI do think that this will result in a weird inconsistency, but fixed anyway.","commit_id":"537d72c3d20571f0552927675b5c8cc8604e3d19"}],"neutron/tests/functional/sanity/test_sanity.py":[{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"bdadc1afcc8cee10866ca252745ae4efc0db5527","unresolved":false,"context_lines":[{"line_number":33,"context_line":"        checks.nova_notify_supported()"},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"    def test_is_mtu_valid_runs(self):"},{"line_number":36,"context_line":"        checks.is_mtu_valid(\u0027127.0.0.1\u0027, [\u0027vxlan\u0027])"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":""},{"line_number":39,"context_line":"class SanityTestCaseRoot(functional_base.BaseSudoTestCase):"}],"source_content_type":"text/x-python","patch_set":7,"id":"3a961159_635c7d4d","line":36,"updated":"2015-01-14 00:36:25.000000000","message":"Do you know if it works for ipv6?  Just curious.  Seems like it should.","commit_id":"a0c572b7a57638424caae7a95d2a6e95231f60ae"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"f03673f0c4fbdaba5ae24e0a1bbd821e97b364f9","unresolved":false,"context_lines":[{"line_number":33,"context_line":"        checks.nova_notify_supported()"},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"    def test_is_mtu_valid_runs(self):"},{"line_number":36,"context_line":"        checks.is_mtu_valid(\u0027127.0.0.1\u0027, [\u0027vxlan\u0027])"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":""},{"line_number":39,"context_line":"class SanityTestCaseRoot(functional_base.BaseSudoTestCase):"}],"source_content_type":"text/x-python","patch_set":7,"id":"3a961159_cf054b7a","line":36,"in_reply_to":"3a961159_2975efcb","updated":"2015-01-14 10:31:33.000000000","message":"I did not check this as I\u0027m not familiar with IPv6 at all, I\u0027m afraid... :(","commit_id":"a0c572b7a57638424caae7a95d2a6e95231f60ae"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"e38876b52a2d6d571a57377932c9d1cd8c8a8d05","unresolved":false,"context_lines":[{"line_number":33,"context_line":"        checks.nova_notify_supported()"},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"    def test_is_mtu_valid_runs(self):"},{"line_number":36,"context_line":"        checks.is_mtu_valid(\u0027127.0.0.1\u0027, [\u0027vxlan\u0027])"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":""},{"line_number":39,"context_line":"class SanityTestCaseRoot(functional_base.BaseSudoTestCase):"}],"source_content_type":"text/x-python","patch_set":7,"id":"3a961159_2975efcb","line":36,"in_reply_to":"3a961159_635c7d4d","updated":"2015-01-14 07:56:13.000000000","message":"This test should be OK, but the other code is probably isn\u0027t?.\n\nIPv6 header is twice as big as IPv4 without options.","commit_id":"a0c572b7a57638424caae7a95d2a6e95231f60ae"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"ed9b4e921190e01fe1c297ab573a4fff32da7187","unresolved":false,"context_lines":[{"line_number":33,"context_line":"        checks.nova_notify_supported()"},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"    def test_is_mtu_valid_runs(self):"},{"line_number":36,"context_line":"        checks.is_mtu_valid(\u0027127.0.0.1\u0027, [\u0027vxlan\u0027])"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":""},{"line_number":39,"context_line":"class SanityTestCaseRoot(functional_base.BaseSudoTestCase):"}],"source_content_type":"text/x-python","patch_set":11,"id":"1a930d6b_7203b2b8","line":36,"updated":"2015-01-21 16:06:36.000000000","message":"if you are using lo then you can read from the file","commit_id":"f8d1e2f5600c7945eb61de09640d60c1ca51cf03"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"1bba4f04c2cb5389a7edea2fc2e549db376d939a","unresolved":false,"context_lines":[{"line_number":33,"context_line":"        checks.nova_notify_supported()"},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"    def test_is_mtu_valid_runs(self):"},{"line_number":36,"context_line":"        checks.is_mtu_valid(\u0027127.0.0.1\u0027, [\u0027vxlan\u0027])"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":""},{"line_number":39,"context_line":"class SanityTestCaseRoot(functional_base.BaseSudoTestCase):"}],"source_content_type":"text/x-python","patch_set":11,"id":"1a930d6b_2cd1c6fb","line":36,"in_reply_to":"1a930d6b_7203b2b8","updated":"2015-01-21 21:42:57.000000000","message":"Which file?","commit_id":"f8d1e2f5600c7945eb61de09640d60c1ca51cf03"}],"neutron/tests/unit/test_sanity_checks.py":[{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"e59fdd5f0b644cf1f1859955f9d862fbf1954d27","unresolved":false,"context_lines":[{"line_number":35,"context_line":""},{"line_number":36,"context_line":"        self.assertFalse(sanity_check.check_mtu())"},{"line_number":37,"context_line":"        self.log_mock.assert_called_once_with("},{"line_number":38,"context_line":"            u\"the local_ip option must be set in either \""},{"line_number":39,"context_line":"            u\"openvswitch\u0027s configuration (under the [ovs] section) \""},{"line_number":40,"context_line":"            u\"and in linuxbridge\u0027s configuration (under the [vxlan] \""},{"line_number":41,"context_line":"            u\"section). if they are both present, they must match.\")"}],"source_content_type":"text/x-python","patch_set":5,"id":"5a890539_d4e1a09b","line":38,"updated":"2014-11-19 23:18:53.000000000","message":"Will need to capitalize to match code. Not sure of the usefulness of checking the log. I know it\u0027s the only way to determine which kind of failure, but it is really brittle.","commit_id":"8e4297b3c96b071a3e251833fb5125fd957d4f7e"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"7c85eca6ae4bdd495e6212b1691cff64c7630b31","unresolved":false,"context_lines":[{"line_number":35,"context_line":""},{"line_number":36,"context_line":"        self.assertFalse(sanity_check.check_mtu())"},{"line_number":37,"context_line":"        self.log_mock.assert_called_once_with("},{"line_number":38,"context_line":"            u\"the local_ip option must be set in either \""},{"line_number":39,"context_line":"            u\"openvswitch\u0027s configuration (under the [ovs] section) \""},{"line_number":40,"context_line":"            u\"and in linuxbridge\u0027s configuration (under the [vxlan] \""},{"line_number":41,"context_line":"            u\"section). if they are both present, they must match.\")"}],"source_content_type":"text/x-python","patch_set":5,"id":"3a961159_416f9d60","line":38,"in_reply_to":"5a890539_d4e1a09b","updated":"2015-01-13 07:59:26.000000000","message":"Done","commit_id":"8e4297b3c96b071a3e251833fb5125fd957d4f7e"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"e59fdd5f0b644cf1f1859955f9d862fbf1954d27","unresolved":false,"context_lines":[{"line_number":40,"context_line":"            u\"and in linuxbridge\u0027s configuration (under the [vxlan] \""},{"line_number":41,"context_line":"            u\"section). if they are both present, they must match.\")"},{"line_number":42,"context_line":""},{"line_number":43,"context_line":"    def test_check_mtu_conflicting_local_ip(self):"},{"line_number":44,"context_line":"        cfg.CONF.OVS.local_ip \u003d \u00271.1.1.1\u0027"},{"line_number":45,"context_line":"        cfg.CONF.VXLAN.local_ip \u003d \u00272.2.2.2\u0027"},{"line_number":46,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"5a890539_94fe3837","line":43,"updated":"2014-11-19 23:18:53.000000000","message":"Maybe add a happy path test case too?","commit_id":"8e4297b3c96b071a3e251833fb5125fd957d4f7e"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"7c85eca6ae4bdd495e6212b1691cff64c7630b31","unresolved":false,"context_lines":[{"line_number":40,"context_line":"            u\"and in linuxbridge\u0027s configuration (under the [vxlan] \""},{"line_number":41,"context_line":"            u\"section). if they are both present, they must match.\")"},{"line_number":42,"context_line":""},{"line_number":43,"context_line":"    def test_check_mtu_conflicting_local_ip(self):"},{"line_number":44,"context_line":"        cfg.CONF.OVS.local_ip \u003d \u00271.1.1.1\u0027"},{"line_number":45,"context_line":"        cfg.CONF.VXLAN.local_ip \u003d \u00272.2.2.2\u0027"},{"line_number":46,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"3a961159_a1976139","line":43,"in_reply_to":"5a890539_94fe3837","updated":"2015-01-13 07:59:26.000000000","message":"I\u0027m never happy.\n\nDone :)","commit_id":"8e4297b3c96b071a3e251833fb5125fd957d4f7e"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"e59fdd5f0b644cf1f1859955f9d862fbf1954d27","unresolved":false,"context_lines":[{"line_number":72,"context_line":""},{"line_number":73,"context_line":"        self.execute_mock.assert_called_once_with("},{"line_number":74,"context_line":"            [\u0027ip\u0027, \u0027addr\u0027, \u0027show\u0027, \u0027to\u0027, FAKE_IP])"},{"line_number":75,"context_line":"        self.log_mock.assert_called_once_with("},{"line_number":76,"context_line":"            u\u0027Could not discover mtu of device owning %s.\u0027, FAKE_IP)"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"    def test_is_mtu_valid_invalid_mtu(self):"}],"source_content_type":"text/x-python","patch_set":5,"id":"5a890539_34b6648b","line":75,"updated":"2014-11-19 23:18:53.000000000","message":"Same concern about checking log, being brittle.","commit_id":"8e4297b3c96b071a3e251833fb5125fd957d4f7e"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"7c85eca6ae4bdd495e6212b1691cff64c7630b31","unresolved":false,"context_lines":[{"line_number":72,"context_line":""},{"line_number":73,"context_line":"        self.execute_mock.assert_called_once_with("},{"line_number":74,"context_line":"            [\u0027ip\u0027, \u0027addr\u0027, \u0027show\u0027, \u0027to\u0027, FAKE_IP])"},{"line_number":75,"context_line":"        self.log_mock.assert_called_once_with("},{"line_number":76,"context_line":"            u\u0027Could not discover mtu of device owning %s.\u0027, FAKE_IP)"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"    def test_is_mtu_valid_invalid_mtu(self):"}],"source_content_type":"text/x-python","patch_set":5,"id":"3a961159_c19aad61","line":75,"in_reply_to":"5a890539_34b6648b","updated":"2015-01-13 07:59:26.000000000","message":"Done","commit_id":"8e4297b3c96b071a3e251833fb5125fd957d4f7e"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"bbff3817a85e7f2211bd9013635f698f34938079","unresolved":false,"context_lines":[{"line_number":27,"context_line":"        self.execute_p \u003d mock.patch(\u0027neutron.agent.linux.utils.execute\u0027)"},{"line_number":28,"context_line":"        self.execute_mock \u003d self.execute_p.start()"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"    def test_check_mtu_no_local_ip(self):"},{"line_number":31,"context_line":"        self.assertEqual(None, checks.get_local_ip(ovs_local_ip\u003dNone,"},{"line_number":32,"context_line":"                                                   vxlan_local_ip\u003dNone))"},{"line_number":33,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"3a961159_05f5d3cb","line":30,"updated":"2015-01-19 22:27:45.000000000","message":"the test method name is incorrect as it test get_local_ip","commit_id":"862edba89d598d97ef3ba6d2aa66d1d372c2de8f"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"dd5532bde674c6db1877d1a179e9bd3504d326aa","unresolved":false,"context_lines":[{"line_number":27,"context_line":"        self.execute_p \u003d mock.patch(\u0027neutron.agent.linux.utils.execute\u0027)"},{"line_number":28,"context_line":"        self.execute_mock \u003d self.execute_p.start()"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"    def test_check_mtu_no_local_ip(self):"},{"line_number":31,"context_line":"        self.assertEqual(None, checks.get_local_ip(ovs_local_ip\u003dNone,"},{"line_number":32,"context_line":"                                                   vxlan_local_ip\u003dNone))"},{"line_number":33,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"3a961159_33f5aecc","line":30,"in_reply_to":"3a961159_05f5d3cb","updated":"2015-01-20 07:49:32.000000000","message":"These comments made me notice that the code didn\u0027t make much sense, so I refactored it a bit. Also fixed the naming scheme for this and the other tests.","commit_id":"862edba89d598d97ef3ba6d2aa66d1d372c2de8f"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"bbff3817a85e7f2211bd9013635f698f34938079","unresolved":false,"context_lines":[{"line_number":31,"context_line":"        self.assertEqual(None, checks.get_local_ip(ovs_local_ip\u003dNone,"},{"line_number":32,"context_line":"                                                   vxlan_local_ip\u003dNone))"},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"    def test_check_mtu_conflicting_local_ip(self):"},{"line_number":35,"context_line":"        self.assertTrue("},{"line_number":36,"context_line":"            checks._is_local_ip_conflicting(ovs_local_ip\u003d\u00271.1.1.1\u0027,"},{"line_number":37,"context_line":"                                            vxlan_local_ip\u003d\u00272.2.2.2\u0027))"}],"source_content_type":"text/x-python","patch_set":9,"id":"3a961159_65fcd7b6","line":34,"updated":"2015-01-19 22:27:45.000000000","message":"itto","commit_id":"862edba89d598d97ef3ba6d2aa66d1d372c2de8f"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"bbff3817a85e7f2211bd9013635f698f34938079","unresolved":false,"context_lines":[{"line_number":36,"context_line":"            checks._is_local_ip_conflicting(ovs_local_ip\u003d\u00271.1.1.1\u0027,"},{"line_number":37,"context_line":"                                            vxlan_local_ip\u003d\u00272.2.2.2\u0027))"},{"line_number":38,"context_line":""},{"line_number":39,"context_line":"    def test_check_mtu_invalid_local_ip(self):"},{"line_number":40,"context_line":"        self.assertTrue("},{"line_number":41,"context_line":"            checks._is_local_ip_conflicting(ovs_local_ip\u003d\u00271.1.1.256\u0027,"},{"line_number":42,"context_line":"                                            vxlan_local_ip\u003d\u00271.1.1.256\u0027))"}],"source_content_type":"text/x-python","patch_set":9,"id":"3a961159_8528e337","line":39,"updated":"2015-01-19 22:27:45.000000000","message":"ditto","commit_id":"862edba89d598d97ef3ba6d2aa66d1d372c2de8f"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"bbff3817a85e7f2211bd9013635f698f34938079","unresolved":false,"context_lines":[{"line_number":41,"context_line":"            checks._is_local_ip_conflicting(ovs_local_ip\u003d\u00271.1.1.256\u0027,"},{"line_number":42,"context_line":"                                            vxlan_local_ip\u003d\u00271.1.1.256\u0027))"},{"line_number":43,"context_line":""},{"line_number":44,"context_line":"    def test_check_mtu(self):"},{"line_number":45,"context_line":"        self.assertEqual(FAKE_IP, checks.get_local_ip(FAKE_IP, FAKE_IP))"},{"line_number":46,"context_line":""},{"line_number":47,"context_line":"    def test_is_mtu_valid(self):"}],"source_content_type":"text/x-python","patch_set":9,"id":"3a961159_051cf313","line":44,"updated":"2015-01-19 22:27:45.000000000","message":"...","commit_id":"862edba89d598d97ef3ba6d2aa66d1d372c2de8f"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"bbff3817a85e7f2211bd9013635f698f34938079","unresolved":false,"context_lines":[{"line_number":48,"context_line":"        self.execute_mock.return_value \u003d \u0027mtu 9000\u0027"},{"line_number":49,"context_line":"        self.assertTrue(checks.is_mtu_valid(FAKE_IP, DEFAULT_TUNNEL_TYPES))"},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"    def test_is_mtu_valid_invalid_ip(self):"},{"line_number":52,"context_line":"        self.execute_mock.return_value \u003d \u0027\u0027"},{"line_number":53,"context_line":"        self.assertEqual(None, checks._get_mtu_of_ip(FAKE_IP))"},{"line_number":54,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"3a961159_6513f701","line":51,"updated":"2015-01-19 22:27:45.000000000","message":"same remark","commit_id":"862edba89d598d97ef3ba6d2aa66d1d372c2de8f"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"dd5532bde674c6db1877d1a179e9bd3504d326aa","unresolved":false,"context_lines":[{"line_number":48,"context_line":"        self.execute_mock.return_value \u003d \u0027mtu 9000\u0027"},{"line_number":49,"context_line":"        self.assertTrue(checks.is_mtu_valid(FAKE_IP, DEFAULT_TUNNEL_TYPES))"},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"    def test_is_mtu_valid_invalid_ip(self):"},{"line_number":52,"context_line":"        self.execute_mock.return_value \u003d \u0027\u0027"},{"line_number":53,"context_line":"        self.assertEqual(None, checks._get_mtu_of_ip(FAKE_IP))"},{"line_number":54,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"3a961159_b39d1e6f","line":51,"in_reply_to":"3a961159_6513f701","updated":"2015-01-20 07:49:32.000000000","message":"Actually here I think the naming scheme is correct","commit_id":"862edba89d598d97ef3ba6d2aa66d1d372c2de8f"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"46e0fe8b802013488337da351396b1b4ea338390","unresolved":false,"context_lines":[{"line_number":1,"context_line":"# Copyright 2014 Red Hat, Inc."},{"line_number":2,"context_line":"#"},{"line_number":3,"context_line":"#    Licensed under the Apache License, Version 2.0 (the \"License\"); you may"},{"line_number":4,"context_line":"#    not use this file except in compliance with the License. You may obtain"}],"source_content_type":"text/x-python","patch_set":10,"id":"1a930d6b_bf641fe6","line":1,"updated":"2015-01-20 21:50:50.000000000","message":"I still completely despise the idea of unit testing sanity checks. They do not do anything complex enough to warrant it, are almost entirely reliant on external system data, and use parts of neutron that are themselves already unit tested. And will happily argue with anyone with a different opinion. :)","commit_id":"537d72c3d20571f0552927675b5c8cc8604e3d19"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"669ebfdd3bd7bbdf2daec7ce45e763903689c0b6","unresolved":false,"context_lines":[{"line_number":1,"context_line":"# Copyright 2014 Red Hat, Inc."},{"line_number":2,"context_line":"#"},{"line_number":3,"context_line":"#    Licensed under the Apache License, Version 2.0 (the \"License\"); you may"},{"line_number":4,"context_line":"#    not use this file except in compliance with the License. You may obtain"}],"source_content_type":"text/x-python","patch_set":10,"id":"1a930d6b_5d7979fb","line":1,"in_reply_to":"1a930d6b_bf641fe6","updated":"2015-01-21 09:39:13.000000000","message":"Normally I would agree, but the code introduced in this patchset contain some non-trivial code that check non-trivial cases. It helped me find some bugs during one of the latest patchsets, so I\u0027d rather keep this here.","commit_id":"537d72c3d20571f0552927675b5c8cc8604e3d19"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"3c0af39cf85f0103ca9e271cd8d291e226ec8e12","unresolved":false,"context_lines":[{"line_number":21,"context_line":"from neutron.plugins.openvswitch.common import config as o_config  # noqa"},{"line_number":22,"context_line":"from neutron.tests import base"},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"FAKE_IP \u003d \u00271.3.3.7\u0027"},{"line_number":25,"context_line":"DEFAULT_TUNNEL_TYPES \u003d [\u0027vxlan\u0027]"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"ba7be1f8_35e68545","line":24,"updated":"2015-02-27 09:30:46.000000000","message":"This violates the RFC that says fun IP addresses must not be used. :)","commit_id":"f8d1e2f5600c7945eb61de09640d60c1ca51cf03"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"50d84bfe0a62b616e269bd71140774330086c5d9","unresolved":false,"context_lines":[{"line_number":21,"context_line":"from neutron.plugins.openvswitch.common import config as o_config  # noqa"},{"line_number":22,"context_line":"from neutron.tests import base"},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"FAKE_IP \u003d \u00271.3.3.7\u0027"},{"line_number":25,"context_line":"DEFAULT_TUNNEL_TYPES \u003d [\u0027vxlan\u0027]"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"ba7be1f8_a63d5c91","line":24,"in_reply_to":"ba7be1f8_35e68545","updated":"2015-03-03 16:29:15.000000000","message":"But... fun! :\u003e","commit_id":"f8d1e2f5600c7945eb61de09640d60c1ca51cf03"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"3c0af39cf85f0103ca9e271cd8d291e226ec8e12","unresolved":false,"context_lines":[{"line_number":34,"context_line":"    def test_get_local_ip_no_ip(self):"},{"line_number":35,"context_line":"        cfg.CONF.OVS.local_ip \u003d None"},{"line_number":36,"context_line":"        cfg.CONF.VXLAN.local_ip \u003d None"},{"line_number":37,"context_line":"        self.assertEqual(None, checks.get_local_ip())"},{"line_number":38,"context_line":""},{"line_number":39,"context_line":"    def test_is_local_ip_valid_conflicting_ips(self):"},{"line_number":40,"context_line":"        cfg.CONF.OVS.local_ip \u003d \u00271.1.1.1\u0027"}],"source_content_type":"text/x-python","patch_set":11,"id":"ba7be1f8_350205b5","line":37,"updated":"2015-02-27 09:30:46.000000000","message":"self.assertIsNone","commit_id":"f8d1e2f5600c7945eb61de09640d60c1ca51cf03"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"50d84bfe0a62b616e269bd71140774330086c5d9","unresolved":false,"context_lines":[{"line_number":34,"context_line":"    def test_get_local_ip_no_ip(self):"},{"line_number":35,"context_line":"        cfg.CONF.OVS.local_ip \u003d None"},{"line_number":36,"context_line":"        cfg.CONF.VXLAN.local_ip \u003d None"},{"line_number":37,"context_line":"        self.assertEqual(None, checks.get_local_ip())"},{"line_number":38,"context_line":""},{"line_number":39,"context_line":"    def test_is_local_ip_valid_conflicting_ips(self):"},{"line_number":40,"context_line":"        cfg.CONF.OVS.local_ip \u003d \u00271.1.1.1\u0027"}],"source_content_type":"text/x-python","patch_set":11,"id":"ba7be1f8_666604bd","line":37,"in_reply_to":"ba7be1f8_350205b5","updated":"2015-03-03 16:29:15.000000000","message":"Done","commit_id":"f8d1e2f5600c7945eb61de09640d60c1ca51cf03"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"3c0af39cf85f0103ca9e271cd8d291e226ec8e12","unresolved":false,"context_lines":[{"line_number":62,"context_line":""},{"line_number":63,"context_line":"    def test_is_mtu_valid_invalid_ip(self):"},{"line_number":64,"context_line":"        self.execute_mock.return_value \u003d \u0027\u0027"},{"line_number":65,"context_line":"        self.assertEqual(None, checks._get_mtu_of_ip(FAKE_IP))"},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"        self.execute_mock.assert_called_once_with("},{"line_number":68,"context_line":"            [\u0027ip\u0027, \u0027addr\u0027, \u0027show\u0027, \u0027to\u0027, FAKE_IP])"}],"source_content_type":"text/x-python","patch_set":11,"id":"ba7be1f8_f51ead0c","line":65,"updated":"2015-02-27 09:30:46.000000000","message":"ditto","commit_id":"f8d1e2f5600c7945eb61de09640d60c1ca51cf03"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"50d84bfe0a62b616e269bd71140774330086c5d9","unresolved":false,"context_lines":[{"line_number":62,"context_line":""},{"line_number":63,"context_line":"    def test_is_mtu_valid_invalid_ip(self):"},{"line_number":64,"context_line":"        self.execute_mock.return_value \u003d \u0027\u0027"},{"line_number":65,"context_line":"        self.assertEqual(None, checks._get_mtu_of_ip(FAKE_IP))"},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"        self.execute_mock.assert_called_once_with("},{"line_number":68,"context_line":"            [\u0027ip\u0027, \u0027addr\u0027, \u0027show\u0027, \u0027to\u0027, FAKE_IP])"}],"source_content_type":"text/x-python","patch_set":11,"id":"ba7be1f8_c60c10ef","line":65,"in_reply_to":"ba7be1f8_f51ead0c","updated":"2015-03-03 16:29:15.000000000","message":"Done","commit_id":"f8d1e2f5600c7945eb61de09640d60c1ca51cf03"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"746cb135f6e43bb5adcd13de3ece0e65484f6cb9","unresolved":false,"context_lines":[{"line_number":13,"context_line":"#    under the License."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"import mock"},{"line_number":16,"context_line":"from oslo.config import cfg"},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"from neutron.cmd.sanity import checks"},{"line_number":19,"context_line":"# Imports for oslo.config"}],"source_content_type":"text/x-python","patch_set":12,"id":"9a80dd14_8c8d6c34","line":16,"updated":"2015-03-04 18:10:30.000000000","message":"Use oslo_config import.","commit_id":"d9047827275bc7387c295d25a83b868a8b900d09"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"105e386a0ae2ac297b1c2ec52dab656eb52773b5","unresolved":false,"context_lines":[{"line_number":13,"context_line":"#    under the License."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"import mock"},{"line_number":16,"context_line":"from oslo.config import cfg"},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"from neutron.cmd.sanity import checks"},{"line_number":19,"context_line":"# Imports for oslo.config"}],"source_content_type":"text/x-python","patch_set":12,"id":"9a80dd14_1edee63f","line":16,"in_reply_to":"9a80dd14_8c8d6c34","updated":"2015-03-05 06:18:11.000000000","message":"Done","commit_id":"d9047827275bc7387c295d25a83b868a8b900d09"},{"author":{"_account_id":10370,"name":"Romil Gupta","email":"romilgupta19@gmail.com","username":"romilg"},"change_message_id":"0ab2d4971c56fc54c36f69faecb03955522267d4","unresolved":false,"context_lines":[{"line_number":16,"context_line":"from oslo_config import cfg"},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"from neutron.cmd.sanity import checks"},{"line_number":19,"context_line":"# Imports for oslo.config"},{"line_number":20,"context_line":"from neutron.plugins.linuxbridge.common import config as l_config  # noqa"},{"line_number":21,"context_line":"from neutron.plugins.openvswitch.common import config as o_config  # noqa"},{"line_number":22,"context_line":"from neutron.tests import base"}],"source_content_type":"text/x-python","patch_set":14,"id":"9a80dd14_59d4d446","line":19,"updated":"2015-03-05 17:57:45.000000000","message":"remove this.","commit_id":"657be02549d3788066145de92e8da8eb8a4e6ce6"},{"author":{"_account_id":10370,"name":"Romil Gupta","email":"romilgupta19@gmail.com","username":"romilg"},"change_message_id":"71169a14e0c3a852ecad559da80b9308cbadb587","unresolved":false,"context_lines":[{"line_number":16,"context_line":"from oslo_config import cfg"},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"from neutron.cmd.sanity import checks"},{"line_number":19,"context_line":"# Imports for oslo.config"},{"line_number":20,"context_line":"from neutron.plugins.linuxbridge.common import config as l_config  # noqa"},{"line_number":21,"context_line":"from neutron.plugins.openvswitch.common import config as o_config  # noqa"},{"line_number":22,"context_line":"from neutron.tests import base"}],"source_content_type":"text/x-python","patch_set":14,"id":"9a80dd14_cb887373","line":19,"in_reply_to":"9a80dd14_2570600a","updated":"2015-03-09 18:45:28.000000000","message":"Ok, no problem :)","commit_id":"657be02549d3788066145de92e8da8eb8a4e6ce6"},{"author":{"_account_id":10370,"name":"Romil Gupta","email":"romilgupta19@gmail.com","username":"romilg"},"change_message_id":"68409a544b0f972a3529e06442827e83f2d21e10","unresolved":false,"context_lines":[{"line_number":16,"context_line":"from oslo_config import cfg"},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"from neutron.cmd.sanity import checks"},{"line_number":19,"context_line":"# Imports for oslo.config"},{"line_number":20,"context_line":"from neutron.plugins.linuxbridge.common import config as l_config  # noqa"},{"line_number":21,"context_line":"from neutron.plugins.openvswitch.common import config as o_config  # noqa"},{"line_number":22,"context_line":"from neutron.tests import base"}],"source_content_type":"text/x-python","patch_set":14,"id":"9a80dd14_5a752fc9","line":19,"in_reply_to":"9a80dd14_37d416eb","updated":"2015-03-09 17:38:57.000000000","message":"What about above suggestion?","commit_id":"657be02549d3788066145de92e8da8eb8a4e6ce6"},{"author":{"_account_id":10370,"name":"Romil Gupta","email":"romilgupta19@gmail.com","username":"romilg"},"change_message_id":"8671e2616cab4e614ed14f3d2e53ffd57882c0ab","unresolved":false,"context_lines":[{"line_number":16,"context_line":"from oslo_config import cfg"},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"from neutron.cmd.sanity import checks"},{"line_number":19,"context_line":"# Imports for oslo.config"},{"line_number":20,"context_line":"from neutron.plugins.linuxbridge.common import config as l_config  # noqa"},{"line_number":21,"context_line":"from neutron.plugins.openvswitch.common import config as o_config  # noqa"},{"line_number":22,"context_line":"from neutron.tests import base"}],"source_content_type":"text/x-python","patch_set":14,"id":"9a80dd14_37d416eb","line":19,"in_reply_to":"9a80dd14_5487c8b4","updated":"2015-03-09 17:11:37.000000000","message":"Kindly, post another patch and remove import statements by doing this way:\n\n   cfg.CONF.import_group(\u0027vxlan\u0027,\u0027neutron.plugins.linuxbridge.common.config\u0027)\n   cfg.CONF.import_group(\u0027ovs\u0027,\u0027neutron.plugins.openvswitch.common.config\u0027)","commit_id":"657be02549d3788066145de92e8da8eb8a4e6ce6"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"abac4cd7af019941e7ae95ac21520e89df4fcf61","unresolved":false,"context_lines":[{"line_number":16,"context_line":"from oslo_config import cfg"},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"from neutron.cmd.sanity import checks"},{"line_number":19,"context_line":"# Imports for oslo.config"},{"line_number":20,"context_line":"from neutron.plugins.linuxbridge.common import config as l_config  # noqa"},{"line_number":21,"context_line":"from neutron.plugins.openvswitch.common import config as o_config  # noqa"},{"line_number":22,"context_line":"from neutron.tests import base"}],"source_content_type":"text/x-python","patch_set":14,"id":"9a80dd14_81574957","line":19,"in_reply_to":"9a80dd14_59d4d446","updated":"2015-03-09 14:23:03.000000000","message":"These are needed for all the cfg.CONF.???.local_ip setup throughout the file, so these imports are needed (if they aren\u0027t here, those lines will fail).","commit_id":"657be02549d3788066145de92e8da8eb8a4e6ce6"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"dd1724ef5b1911462c2822f4551b9921fdb79200","unresolved":false,"context_lines":[{"line_number":16,"context_line":"from oslo_config import cfg"},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"from neutron.cmd.sanity import checks"},{"line_number":19,"context_line":"# Imports for oslo.config"},{"line_number":20,"context_line":"from neutron.plugins.linuxbridge.common import config as l_config  # noqa"},{"line_number":21,"context_line":"from neutron.plugins.openvswitch.common import config as o_config  # noqa"},{"line_number":22,"context_line":"from neutron.tests import base"}],"source_content_type":"text/x-python","patch_set":14,"id":"9a80dd14_2570600a","line":19,"in_reply_to":"9a80dd14_5a752fc9","updated":"2015-03-09 18:07:22.000000000","message":"I normally post \u0027Done\u0027 when I implement the code. I didn\u0027t implement the code yet so it\u0027s in my todo list :)","commit_id":"657be02549d3788066145de92e8da8eb8a4e6ce6"},{"author":{"_account_id":10370,"name":"Romil Gupta","email":"romilgupta19@gmail.com","username":"romilg"},"change_message_id":"58650fde948a5e8d2ec963541c050ad74ad4594a","unresolved":false,"context_lines":[{"line_number":16,"context_line":"from oslo_config import cfg"},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"from neutron.cmd.sanity import checks"},{"line_number":19,"context_line":"# Imports for oslo.config"},{"line_number":20,"context_line":"from neutron.plugins.linuxbridge.common import config as l_config  # noqa"},{"line_number":21,"context_line":"from neutron.plugins.openvswitch.common import config as o_config  # noqa"},{"line_number":22,"context_line":"from neutron.tests import base"}],"source_content_type":"text/x-python","patch_set":14,"id":"9a80dd14_92c79595","line":19,"in_reply_to":"9a80dd14_81574957","updated":"2015-03-09 15:36:37.000000000","message":"Actually, I asked to remove this line:\n\n# Imports for oslo.config","commit_id":"657be02549d3788066145de92e8da8eb8a4e6ce6"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"33bdf7b42a916660c98671f71aa603dc05825609","unresolved":false,"context_lines":[{"line_number":16,"context_line":"from oslo_config import cfg"},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"from neutron.cmd.sanity import checks"},{"line_number":19,"context_line":"# Imports for oslo.config"},{"line_number":20,"context_line":"from neutron.plugins.linuxbridge.common import config as l_config  # noqa"},{"line_number":21,"context_line":"from neutron.plugins.openvswitch.common import config as o_config  # noqa"},{"line_number":22,"context_line":"from neutron.tests import base"}],"source_content_type":"text/x-python","patch_set":14,"id":"9a80dd14_ed71d40c","line":19,"in_reply_to":"9a80dd14_92c79595","updated":"2015-03-09 16:09:10.000000000","message":"ah... but the imports are for oslo.config so I\u0027d rather keep them (avoid people asking why these are here :)","commit_id":"657be02549d3788066145de92e8da8eb8a4e6ce6"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"3152d37a23dfd1a19d88e86770b31681cf63c9dd","unresolved":false,"context_lines":[{"line_number":16,"context_line":"from oslo_config import cfg"},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"from neutron.cmd.sanity import checks"},{"line_number":19,"context_line":"# Imports for oslo.config"},{"line_number":20,"context_line":"from neutron.plugins.linuxbridge.common import config as l_config  # noqa"},{"line_number":21,"context_line":"from neutron.plugins.openvswitch.common import config as o_config  # noqa"},{"line_number":22,"context_line":"from neutron.tests import base"}],"source_content_type":"text/x-python","patch_set":14,"id":"9a80dd14_5487c8b4","line":19,"in_reply_to":"9a80dd14_b477249f","updated":"2015-03-09 16:46:23.000000000","message":"I disagree regarding removing the comment. It doesn\u0027t hurt to be a bit more explicit.\n\nRegarding the changes to the names - I agree your suggestion is better but I consider it a nit. If I end up making another revision I will surely make this change, but I don\u0027t want to add another patchset specifically for this. Is this fine with you?\n\nRegarding the \"# noqa\" comments - since l_config and o_config aren\u0027t used anywhere in the code (they are only used so oslo.config will work), pep8 checks raise this as \u0027unused imports\u0027. # noqa tells the checker to ignore these errors.","commit_id":"657be02549d3788066145de92e8da8eb8a4e6ce6"},{"author":{"_account_id":12444,"name":"John Schwarz","email":"jschwarz@redhat.com","username":"jschwarz"},"change_message_id":"f4161b96e238a088628ed92198b9334e23e00bd8","unresolved":false,"context_lines":[{"line_number":16,"context_line":"from oslo_config import cfg"},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"from neutron.cmd.sanity import checks"},{"line_number":19,"context_line":"# Imports for oslo.config"},{"line_number":20,"context_line":"from neutron.plugins.linuxbridge.common import config as l_config  # noqa"},{"line_number":21,"context_line":"from neutron.plugins.openvswitch.common import config as o_config  # noqa"},{"line_number":22,"context_line":"from neutron.tests import base"}],"source_content_type":"text/x-python","patch_set":14,"id":"9a80dd14_d4ca4b45","line":19,"in_reply_to":"9a80dd14_cb887373","updated":"2015-03-18 10:03:51.000000000","message":"Done :)","commit_id":"657be02549d3788066145de92e8da8eb8a4e6ce6"},{"author":{"_account_id":10370,"name":"Romil Gupta","email":"romilgupta19@gmail.com","username":"romilg"},"change_message_id":"1180f59d410935c56322178a64f2d8a53eeae691","unresolved":false,"context_lines":[{"line_number":16,"context_line":"from oslo_config import cfg"},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"from neutron.cmd.sanity import checks"},{"line_number":19,"context_line":"# Imports for oslo.config"},{"line_number":20,"context_line":"from neutron.plugins.linuxbridge.common import config as l_config  # noqa"},{"line_number":21,"context_line":"from neutron.plugins.openvswitch.common import config as o_config  # noqa"},{"line_number":22,"context_line":"from neutron.tests import base"}],"source_content_type":"text/x-python","patch_set":14,"id":"9a80dd14_b477249f","line":19,"in_reply_to":"9a80dd14_ed71d40c","updated":"2015-03-09 16:29:23.000000000","message":"It\u0027s implicit no need to specify.\n# Imports for oslo.config \n\nAlso, \nI would suggest to make this changes,\n\nfrom neutron.plugins.linuxbridge.common import config as lb_config\nfrom neutron.plugins.openvswitch.common import config as ovs_config\n\nAnd, what do we mean by this comment \"# noqa\" ???","commit_id":"657be02549d3788066145de92e8da8eb8a4e6ce6"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"74a0db73f09ec175235a1e9ef163b411615942a3","unresolved":false,"context_lines":[{"line_number":16,"context_line":"from oslo_config import cfg"},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"from neutron.cmd.sanity import checks"},{"line_number":19,"context_line":"# Imports for oslo.config"},{"line_number":20,"context_line":"from neutron.tests import base"},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"FAKE_IP \u003d \u00271.3.3.7\u0027"}],"source_content_type":"text/x-python","patch_set":16,"id":"9a80dd14_b2edd090","line":19,"updated":"2015-03-18 18:12:29.000000000","message":"Comment doesn\u0027t make sense here...","commit_id":"a27fad82f6acf55c53c06999dd2a2d97cb61ffab"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"74a0db73f09ec175235a1e9ef163b411615942a3","unresolved":false,"context_lines":[{"line_number":63,"context_line":""},{"line_number":64,"context_line":"    def test_is_mtu_valid_invalid_ip(self):"},{"line_number":65,"context_line":"        self.execute_mock.return_value \u003d \u0027\u0027"},{"line_number":66,"context_line":"        self.assertIsNone(None, checks._get_mtu_of_ip(FAKE_IP))"},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"        self.execute_mock.assert_called_once_with("},{"line_number":69,"context_line":"            [\u0027ip\u0027, \u0027addr\u0027, \u0027show\u0027, \u0027to\u0027, FAKE_IP])"}],"source_content_type":"text/x-python","patch_set":16,"id":"9a80dd14_724d18c4","line":66,"updated":"2015-03-18 18:12:29.000000000","message":"self.assertIsNone(checks._get_mtu_of_ip(FAKE_IP))","commit_id":"a27fad82f6acf55c53c06999dd2a2d97cb61ffab"}]}