)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"a3c261d43caa5bc1b10b2df0a623dd2030d0c3a4","unresolved":false,"context_lines":[{"line_number":9,"context_line":"(Patch set #7 for the multiple-ipv6-prefixes blueprint)"},{"line_number":10,"context_line":""},{"line_number":11,"context_line":"This patch set prevents calls to _allocate_specific_ip for"},{"line_number":12,"context_line":"requests of a specific IPv6 SLAAC address assignment on an"},{"line_number":13,"context_line":"internal router port. In this case, all that is required"},{"line_number":14,"context_line":"is a uniqueness check on the requested address, as is"},{"line_number":15,"context_line":"already being done in _test_fixed_ips_for_port."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"1a6ced46_8f6e0195","line":12,"updated":"2015-03-24 19:27:52.000000000","message":"I\u0027m struggling to understand what a \"specific IPv6 SLAAC address assignment\" is.  Why would one request a specific slaac address?  If it understand correctly, there is only one slaac address for a mac address.  Maybe as I review, I\u0027ll understand better.","commit_id":"9add7392301ec0c97941ac232dace477b44f5590"},{"author":{"_account_id":6695,"name":"Dane LeBlanc","email":"leblancd@cisco.com","username":"leblancd"},"change_message_id":"a970193f456e85ee65e95d0817a36d73394c44c7","unresolved":false,"context_lines":[{"line_number":9,"context_line":"(Patch set #7 for the multiple-ipv6-prefixes blueprint)"},{"line_number":10,"context_line":""},{"line_number":11,"context_line":"This patch set prevents calls to _allocate_specific_ip for"},{"line_number":12,"context_line":"requests of a specific IPv6 SLAAC address assignment on an"},{"line_number":13,"context_line":"internal router port. In this case, all that is required"},{"line_number":14,"context_line":"is a uniqueness check on the requested address, as is"},{"line_number":15,"context_line":"already being done in _test_fixed_ips_for_port."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"fa963961_87b46ff2","line":12,"in_reply_to":"1a6ced46_8f6e0195","updated":"2015-03-27 20:19:56.000000000","message":"On internal router ports, addresses can be assigned for SLAAC subnets that don\u0027t conform to EUI-64. Typically, these addresses end in a convenient small integer, e.g. ::1 or ::2. The reason for this is that these addresses are the subnet gateway addresses that a tenant supplies when creating a subnet.  Because these addresses can be chosen arbitrarily, we need to have uniqueness checks to make sure that someone doesn\u0027t use an address that just happens to co-incide with an EUI-64 address that will be used on an internal port (very remote chance of this happening, but it could happen).","commit_id":"9add7392301ec0c97941ac232dace477b44f5590"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"fc30e57746f1cce1724334b3731edc7aa1bc2052","unresolved":false,"context_lines":[{"line_number":9,"context_line":"(Patch set #7 for the multiple-ipv6-prefixes blueprint)"},{"line_number":10,"context_line":""},{"line_number":11,"context_line":"This patch set prevents calls to _allocate_specific_ip for"},{"line_number":12,"context_line":"requests of a specific IPv6 SLAAC address assignment on an"},{"line_number":13,"context_line":"internal router port. In this case, all that is required"},{"line_number":14,"context_line":"is a uniqueness check on the requested address, as is"},{"line_number":15,"context_line":"already being done in _test_fixed_ips_for_port."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"da9b358b_a8505b5f","line":12,"in_reply_to":"fa963961_87b46ff2","updated":"2015-03-31 04:49:02.000000000","message":"Okay, so it a specific address assignment on a subnet that is otherwise assigned with EUI-64.  Thank you for the clarification.\n\nI still wonder about the need to check uniqueness with EUI-64.  I think it would better to prevent specific address allocations from the EUI-64 space.  But, that might be out of the scope of this patch.  Something to think about for later.","commit_id":"9add7392301ec0c97941ac232dace477b44f5590"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"39d04e2b19fe1f82b1b0c69b95e1d243d18e5b9d","unresolved":false,"context_lines":[{"line_number":18,"context_line":"(non-EUI-64) address for a SLAAC subnet, the call flow includes"},{"line_number":19,"context_line":"a call to _allocate_specific_ip. This call is not necessary,"},{"line_number":20,"context_line":"since we\u0027re not allocating an address from a pool (and"},{"line_number":21,"context_line":"recalibrating availability ranges, etc.)."},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"This patch set prevents the call to _allocate_specific_ip for"},{"line_number":24,"context_line":"this scenario."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"da9b358b_48642f21","line":21,"updated":"2015-03-31 04:52:49.000000000","message":"This description is much improved.  Thank you for the clarification.","commit_id":"04cd03840977bc32b2bcadcd185a8c4ae19b7159"}],"neutron/db/db_base_plugin_v2.py":[{"author":{"_account_id":14844,"name":"John Joyce","email":"joycej@cisco.com","username":"john_a_joyce"},"change_message_id":"f6e7f205f0b1bde9e471005e3083f45cf04769bf","unresolved":false,"context_lines":[{"line_number":525,"context_line":"            # auto address subnets (only allowed for router ports)"},{"line_number":526,"context_line":"            ips +\u003d fixed_auto_addr"},{"line_number":527,"context_line":""},{"line_number":528,"context_line":"            # Implicitly include all auto-address subnets for address"},{"line_number":529,"context_line":"            # except for subnets with a specific IP address assignment."},{"line_number":530,"context_line":"            fixed_auto_subnet_ids \u003d [fixed_ip[\u0027subnet_id\u0027] for fixed_ip"},{"line_number":531,"context_line":"                                     in fixed_auto_addr]"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a80dd14_c8afc729","line":528,"updated":"2015-03-19 15:03:15.000000000","message":"I think you should only be doing this for non-router ports.  IOW keep similar logic to the original code for adding this subnet to v6_stateless","commit_id":"ddb3b1d3c4a6d7a9cd89e19b4b43482ba97c3ada"},{"author":{"_account_id":6695,"name":"Dane LeBlanc","email":"leblancd@cisco.com","username":"leblancd"},"change_message_id":"62f91dce4d6fd73b7505e0925d7c7e89bf32e418","unresolved":false,"context_lines":[{"line_number":525,"context_line":"            # auto address subnets (only allowed for router ports)"},{"line_number":526,"context_line":"            ips +\u003d fixed_auto_addr"},{"line_number":527,"context_line":""},{"line_number":528,"context_line":"            # Implicitly include all auto-address subnets for address"},{"line_number":529,"context_line":"            # except for subnets with a specific IP address assignment."},{"line_number":530,"context_line":"            fixed_auto_subnet_ids \u003d [fixed_ip[\u0027subnet_id\u0027] for fixed_ip"},{"line_number":531,"context_line":"                                     in fixed_auto_addr]"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a80dd14_55ac4b88","line":528,"in_reply_to":"9a80dd14_c8afc729","updated":"2015-03-19 16:03:46.000000000","message":"Done","commit_id":"ddb3b1d3c4a6d7a9cd89e19b4b43482ba97c3ada"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"a3c261d43caa5bc1b10b2df0a623dd2030d0c3a4","unresolved":false,"context_lines":[{"line_number":443,"context_line":"            raise n_exc.InvalidInput(error_message\u003dmsg)"},{"line_number":444,"context_line":"        return fixed_ip_set"},{"line_number":445,"context_line":""},{"line_number":446,"context_line":"    def _allocate_fixed_ips(self, context, fixed_ips, mac_address):"},{"line_number":447,"context_line":"        \"\"\"Allocate IP addresses according to the configured fixed_ips.\"\"\""},{"line_number":448,"context_line":"        ips \u003d []"},{"line_number":449,"context_line":"        for fixed in fixed_ips:"}],"source_content_type":"text/x-python","patch_set":7,"id":"1a6ced46_4b8cfe35","side":"PARENT","line":446,"updated":"2015-03-24 19:27:52.000000000","message":"I can\u0027t say that I\u0027m sad to see mac_address no longer involved in allocating a fixed ip.","commit_id":"906ede185c0ddba4e994f41dacbd96d845de4bde"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"a3c261d43caa5bc1b10b2df0a623dd2030d0c3a4","unresolved":false,"context_lines":[{"line_number":463,"context_line":"            prefix \u003d subnet[\u0027cidr\u0027]"},{"line_number":464,"context_line":"            ip_address \u003d ipv6_utils.get_ipv6_addr_by_EUI64(prefix,"},{"line_number":465,"context_line":"                                                           port[\u0027mac_address\u0027])"},{"line_number":466,"context_line":"            if not self._check_unique_ip(context, port[\u0027network_id\u0027],"},{"line_number":467,"context_line":"                                         subnet[\u0027id\u0027], ip_address.format()):"},{"line_number":468,"context_line":"                raise n_exc.IpAddressInUse(net_id\u003dport[\u0027network_id\u0027],"},{"line_number":469,"context_line":"                                           ip_address\u003dip_address.format())"}],"source_content_type":"text/x-python","patch_set":7,"id":"1a6ced46_0fa691c1","line":466,"updated":"2015-03-24 19:27:52.000000000","message":"Why would a slaac address not be unique?  We need the mac to be unique and the eui64 address is just a mapping from the mac address.  Do we really need this check?\n\nI guess my comment here is probably out of the scope of this review.  So, feel free to disregard it.","commit_id":"9add7392301ec0c97941ac232dace477b44f5590"},{"author":{"_account_id":6695,"name":"Dane LeBlanc","email":"leblancd@cisco.com","username":"leblancd"},"change_message_id":"a970193f456e85ee65e95d0817a36d73394c44c7","unresolved":false,"context_lines":[{"line_number":463,"context_line":"            prefix \u003d subnet[\u0027cidr\u0027]"},{"line_number":464,"context_line":"            ip_address \u003d ipv6_utils.get_ipv6_addr_by_EUI64(prefix,"},{"line_number":465,"context_line":"                                                           port[\u0027mac_address\u0027])"},{"line_number":466,"context_line":"            if not self._check_unique_ip(context, port[\u0027network_id\u0027],"},{"line_number":467,"context_line":"                                         subnet[\u0027id\u0027], ip_address.format()):"},{"line_number":468,"context_line":"                raise n_exc.IpAddressInUse(net_id\u003dport[\u0027network_id\u0027],"},{"line_number":469,"context_line":"                                           ip_address\u003dip_address.format())"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa963961_e7d8db1f","line":466,"in_reply_to":"1a6ced46_0fa691c1","updated":"2015-03-27 20:19:56.000000000","message":"See my response for the commit message. There is a very remote chance that there could be overlap. This uniqueness check is in the current upstream code. We can remove this in a future patch if the consensus is that this chance is so remote that this uniqueness check isn\u0027t warranted.","commit_id":"9add7392301ec0c97941ac232dace477b44f5590"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"fc30e57746f1cce1724334b3731edc7aa1bc2052","unresolved":false,"context_lines":[{"line_number":463,"context_line":"            prefix \u003d subnet[\u0027cidr\u0027]"},{"line_number":464,"context_line":"            ip_address \u003d ipv6_utils.get_ipv6_addr_by_EUI64(prefix,"},{"line_number":465,"context_line":"                                                           port[\u0027mac_address\u0027])"},{"line_number":466,"context_line":"            if not self._check_unique_ip(context, port[\u0027network_id\u0027],"},{"line_number":467,"context_line":"                                         subnet[\u0027id\u0027], ip_address.format()):"},{"line_number":468,"context_line":"                raise n_exc.IpAddressInUse(net_id\u003dport[\u0027network_id\u0027],"},{"line_number":469,"context_line":"                                           ip_address\u003dip_address.format())"}],"source_content_type":"text/x-python","patch_set":7,"id":"da9b358b_48ef8f87","line":466,"in_reply_to":"fa963961_e7d8db1f","updated":"2015-03-31 04:49:02.000000000","message":"ok","commit_id":"9add7392301ec0c97941ac232dace477b44f5590"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"a3c261d43caa5bc1b10b2df0a623dd2030d0c3a4","unresolved":false,"context_lines":[{"line_number":471,"context_line":"                        \u0027subnet_id\u0027: subnet[\u0027id\u0027]})"},{"line_number":472,"context_line":"        return ips"},{"line_number":473,"context_line":""},{"line_number":474,"context_line":"    def _process_fixed_ips(self, context, port, device_owner, fixed_ips,"},{"line_number":475,"context_line":"                           subnets\u003dNone, for_create\u003dFalse):"},{"line_number":476,"context_line":"        ips \u003d []"},{"line_number":477,"context_line":"        alloc_ips \u003d []"}],"source_content_type":"text/x-python","patch_set":7,"id":"1a6ced46_12523613","line":474,"updated":"2015-03-24 19:27:52.000000000","message":"This method name doesn\u0027t really help me know what is going on here.  I know we use \u0027process\u0027 all over the place but it isn\u0027t really doing it for me here.\n\nMaybe something like _identify_all_fixed_ips or _allocate_or_generate_fixed_ips","commit_id":"9add7392301ec0c97941ac232dace477b44f5590"},{"author":{"_account_id":6695,"name":"Dane LeBlanc","email":"leblancd@cisco.com","username":"leblancd"},"change_message_id":"a970193f456e85ee65e95d0817a36d73394c44c7","unresolved":false,"context_lines":[{"line_number":471,"context_line":"                        \u0027subnet_id\u0027: subnet[\u0027id\u0027]})"},{"line_number":472,"context_line":"        return ips"},{"line_number":473,"context_line":""},{"line_number":474,"context_line":"    def _process_fixed_ips(self, context, port, device_owner, fixed_ips,"},{"line_number":475,"context_line":"                           subnets\u003dNone, for_create\u003dFalse):"},{"line_number":476,"context_line":"        ips \u003d []"},{"line_number":477,"context_line":"        alloc_ips \u003d []"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa963961_87834fcc","line":474,"in_reply_to":"1a6ced46_12523613","updated":"2015-03-27 20:19:56.000000000","message":"We were doing a bit of refactoring here along with the fix. I\u0027ll submit a new (minimalized) patch with just the fix. (Refactoring can be done separately if needed).","commit_id":"9add7392301ec0c97941ac232dace477b44f5590"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"fc30e57746f1cce1724334b3731edc7aa1bc2052","unresolved":false,"context_lines":[{"line_number":471,"context_line":"                        \u0027subnet_id\u0027: subnet[\u0027id\u0027]})"},{"line_number":472,"context_line":"        return ips"},{"line_number":473,"context_line":""},{"line_number":474,"context_line":"    def _process_fixed_ips(self, context, port, device_owner, fixed_ips,"},{"line_number":475,"context_line":"                           subnets\u003dNone, for_create\u003dFalse):"},{"line_number":476,"context_line":"        ips \u003d []"},{"line_number":477,"context_line":"        alloc_ips \u003d []"}],"source_content_type":"text/x-python","patch_set":7,"id":"da9b358b_88fd973c","line":474,"in_reply_to":"fa963961_87834fcc","updated":"2015-03-31 04:49:02.000000000","message":"Yes, I like that.  As a reviewer, I find there is more burden to reviewing a patch with more than one thing getting done than there is to review to separate but very focused patches.","commit_id":"9add7392301ec0c97941ac232dace477b44f5590"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"a3c261d43caa5bc1b10b2df0a623dd2030d0c3a4","unresolved":false,"context_lines":[{"line_number":476,"context_line":"        ips \u003d []"},{"line_number":477,"context_line":"        alloc_ips \u003d []"},{"line_number":478,"context_line":"        slaac_subnets \u003d []"},{"line_number":479,"context_line":"        for fip in fixed_ips:"},{"line_number":480,"context_line":"            subnet \u003d self._get_subnet(context, fip[\u0027subnet_id\u0027])"},{"line_number":481,"context_line":"            if ipv6_utils.is_auto_address_subnet(subnet):"},{"line_number":482,"context_line":"                # For a router port with a preassigned ip on a slaac subnet,"}],"source_content_type":"text/x-python","patch_set":7,"id":"1a6ced46_32f572ee","line":479,"updated":"2015-03-24 19:27:52.000000000","message":"nit:  Could you use fixed_ip instead of fip?  fip is often used for floating ip.  I know that wouldn\u0027t make sense in this context but still fixed_ip would help readability a little.  It is a nit, so don\u0027t worry about it too much.","commit_id":"9add7392301ec0c97941ac232dace477b44f5590"},{"author":{"_account_id":6695,"name":"Dane LeBlanc","email":"leblancd@cisco.com","username":"leblancd"},"change_message_id":"a970193f456e85ee65e95d0817a36d73394c44c7","unresolved":false,"context_lines":[{"line_number":476,"context_line":"        ips \u003d []"},{"line_number":477,"context_line":"        alloc_ips \u003d []"},{"line_number":478,"context_line":"        slaac_subnets \u003d []"},{"line_number":479,"context_line":"        for fip in fixed_ips:"},{"line_number":480,"context_line":"            subnet \u003d self._get_subnet(context, fip[\u0027subnet_id\u0027])"},{"line_number":481,"context_line":"            if ipv6_utils.is_auto_address_subnet(subnet):"},{"line_number":482,"context_line":"                # For a router port with a preassigned ip on a slaac subnet,"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa963961_476e6730","line":479,"in_reply_to":"1a6ced46_32f572ee","updated":"2015-03-27 20:19:56.000000000","message":"This method will be removed in the next patch.","commit_id":"9add7392301ec0c97941ac232dace477b44f5590"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"a3c261d43caa5bc1b10b2df0a623dd2030d0c3a4","unresolved":false,"context_lines":[{"line_number":490,"context_line":"                        ips.append(fip)"},{"line_number":491,"context_line":"                    else:"},{"line_number":492,"context_line":"                        slaac_subnets.append(subnet)"},{"line_number":493,"context_line":"            else:"},{"line_number":494,"context_line":"                alloc_ips.append(fip)"},{"line_number":495,"context_line":""},{"line_number":496,"context_line":"        # For create on regular ports, add slaac subnets"}],"source_content_type":"text/x-python","patch_set":7,"id":"1a6ced46_78e8a9e2","line":493,"updated":"2015-03-24 19:27:52.000000000","message":"nit:  I prefer to stick really short else bodies first.  It is just a readability thing.\n\nif not ipv6_utils.is_auto_address_subnet(subnet):\n    alloc_ips.append(fip)\nelse:\n    ... big else block here ...","commit_id":"9add7392301ec0c97941ac232dace477b44f5590"},{"author":{"_account_id":6695,"name":"Dane LeBlanc","email":"leblancd@cisco.com","username":"leblancd"},"change_message_id":"a970193f456e85ee65e95d0817a36d73394c44c7","unresolved":false,"context_lines":[{"line_number":490,"context_line":"                        ips.append(fip)"},{"line_number":491,"context_line":"                    else:"},{"line_number":492,"context_line":"                        slaac_subnets.append(subnet)"},{"line_number":493,"context_line":"            else:"},{"line_number":494,"context_line":"                alloc_ips.append(fip)"},{"line_number":495,"context_line":""},{"line_number":496,"context_line":"        # For create on regular ports, add slaac subnets"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa963961_e7745bfe","line":493,"in_reply_to":"1a6ced46_78e8a9e2","updated":"2015-03-27 20:19:56.000000000","message":"This method will be removed in next patch.","commit_id":"9add7392301ec0c97941ac232dace477b44f5590"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"a3c261d43caa5bc1b10b2df0a623dd2030d0c3a4","unresolved":false,"context_lines":[{"line_number":497,"context_line":"        if (for_create and"},{"line_number":498,"context_line":"            device_owner not in constants.ROUTER_INTERFACE_OWNERS and"},{"line_number":499,"context_line":"            device_owner !\u003d constants.DEVICE_OWNER_ROUTER_SNAT):"},{"line_number":500,"context_line":"            slaac_subnets +\u003d [s for s in subnets"},{"line_number":501,"context_line":"                              if ipv6_utils.is_auto_address_subnet(s)]"},{"line_number":502,"context_line":""},{"line_number":503,"context_line":"        if alloc_ips:"}],"source_content_type":"text/x-python","patch_set":7,"id":"1a6ced46_e3514ed9","line":500,"updated":"2015-03-24 19:27:52.000000000","message":"It feels a little like this method could be split in to two methods to be a little more understandable.  The first method would leave off the subnet argument.  The second method would follow up by adding ips for these subnets passed in to the method as \"subnets\".  Does that make sense?\n\nThis may make the for_create option obsolete too.  I don\u0027t like passing booleans in to a method to control things like this.  It always makes me think that there are two methods in here.","commit_id":"9add7392301ec0c97941ac232dace477b44f5590"},{"author":{"_account_id":6695,"name":"Dane LeBlanc","email":"leblancd@cisco.com","username":"leblancd"},"change_message_id":"a970193f456e85ee65e95d0817a36d73394c44c7","unresolved":false,"context_lines":[{"line_number":497,"context_line":"        if (for_create and"},{"line_number":498,"context_line":"            device_owner not in constants.ROUTER_INTERFACE_OWNERS and"},{"line_number":499,"context_line":"            device_owner !\u003d constants.DEVICE_OWNER_ROUTER_SNAT):"},{"line_number":500,"context_line":"            slaac_subnets +\u003d [s for s in subnets"},{"line_number":501,"context_line":"                              if ipv6_utils.is_auto_address_subnet(s)]"},{"line_number":502,"context_line":""},{"line_number":503,"context_line":"        if alloc_ips:"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa963961_27524368","line":500,"in_reply_to":"1a6ced46_e3514ed9","updated":"2015-03-27 20:19:56.000000000","message":"This method will be removed in the next patch.","commit_id":"9add7392301ec0c97941ac232dace477b44f5590"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"a3c261d43caa5bc1b10b2df0a623dd2030d0c3a4","unresolved":false,"context_lines":[{"line_number":502,"context_line":""},{"line_number":503,"context_line":"        if alloc_ips:"},{"line_number":504,"context_line":"            if for_create:"},{"line_number":505,"context_line":"                LOG.debug(\"Port create. Adding %s\", alloc_ips)"},{"line_number":506,"context_line":"            else:"},{"line_number":507,"context_line":"                LOG.debug(\"Port update. Adding %s\", alloc_ips)"},{"line_number":508,"context_line":"            ips +\u003d self._allocate_fixed_ips(context, alloc_ips)"}],"source_content_type":"text/x-python","patch_set":7,"id":"1a6ced46_de19857a","line":505,"updated":"2015-03-24 19:27:52.000000000","message":"nit:  Can\u0027t we just find a message that makes sense in both contexts?","commit_id":"9add7392301ec0c97941ac232dace477b44f5590"},{"author":{"_account_id":6695,"name":"Dane LeBlanc","email":"leblancd@cisco.com","username":"leblancd"},"change_message_id":"a970193f456e85ee65e95d0817a36d73394c44c7","unresolved":false,"context_lines":[{"line_number":502,"context_line":""},{"line_number":503,"context_line":"        if alloc_ips:"},{"line_number":504,"context_line":"            if for_create:"},{"line_number":505,"context_line":"                LOG.debug(\"Port create. Adding %s\", alloc_ips)"},{"line_number":506,"context_line":"            else:"},{"line_number":507,"context_line":"                LOG.debug(\"Port update. Adding %s\", alloc_ips)"},{"line_number":508,"context_line":"            ips +\u003d self._allocate_fixed_ips(context, alloc_ips)"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa963961_c7c71711","line":505,"in_reply_to":"1a6ced46_de19857a","updated":"2015-03-27 20:19:56.000000000","message":"Ditto.","commit_id":"9add7392301ec0c97941ac232dace477b44f5590"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"a3c261d43caa5bc1b10b2df0a623dd2030d0c3a4","unresolved":false,"context_lines":[{"line_number":511,"context_line":"            ips +\u003d self._get_slaac_ips(context, port, slaac_subnets)"},{"line_number":512,"context_line":"        return ips"},{"line_number":513,"context_line":""},{"line_number":514,"context_line":"    def _update_ips_for_port(self, context, port, network_id, original_ips,"},{"line_number":515,"context_line":"                             new_ips, device_owner):"},{"line_number":516,"context_line":"        \"\"\"Add or remove IPs from the port.\"\"\""},{"line_number":517,"context_line":"        ips \u003d []"}],"source_content_type":"text/x-python","patch_set":7,"id":"1a6ced46_f0811790","line":514,"updated":"2015-03-24 19:27:52.000000000","message":"Note:  Adds port argument.  Removes port_id, and mac_address (presumably because they can be obtained from the port).","commit_id":"9add7392301ec0c97941ac232dace477b44f5590"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"a3c261d43caa5bc1b10b2df0a623dd2030d0c3a4","unresolved":false,"context_lines":[{"line_number":536,"context_line":"                # For ports that are not router ports, retain any automatic"},{"line_number":537,"context_line":"                # (non-optional, e.g. IPv6 SLAAC) addresses."},{"line_number":538,"context_line":"                if (device_owner not in constants.ROUTER_INTERFACE_OWNERS and"},{"line_number":539,"context_line":"                    device_owner !\u003d constants.DEVICE_OWNER_ROUTER_SNAT):"},{"line_number":540,"context_line":"                    subnet \u003d self._get_subnet(context,"},{"line_number":541,"context_line":"                                              original_ip[\u0027subnet_id\u0027])"},{"line_number":542,"context_line":"                    if (ipv6_utils.is_auto_address_subnet(subnet)):"}],"source_content_type":"text/x-python","patch_set":7,"id":"1a6ced46_e3edcef0","line":539,"updated":"2015-03-24 19:27:52.000000000","message":"Was it a bug that this wasn\u0027t checked before?  This addition seems orthogonal to the rest of the patch.  Maybe I\u0027m wrong.\n\nThis makes me wonder why DEVICE_OWNER_ROUTER_SNAT not included in ROUTER_INTERFACE_OWNERS in the first place but that might be out of the scope of this patch.","commit_id":"9add7392301ec0c97941ac232dace477b44f5590"},{"author":{"_account_id":6695,"name":"Dane LeBlanc","email":"leblancd@cisco.com","username":"leblancd"},"change_message_id":"a970193f456e85ee65e95d0817a36d73394c44c7","unresolved":false,"context_lines":[{"line_number":536,"context_line":"                # For ports that are not router ports, retain any automatic"},{"line_number":537,"context_line":"                # (non-optional, e.g. IPv6 SLAAC) addresses."},{"line_number":538,"context_line":"                if (device_owner not in constants.ROUTER_INTERFACE_OWNERS and"},{"line_number":539,"context_line":"                    device_owner !\u003d constants.DEVICE_OWNER_ROUTER_SNAT):"},{"line_number":540,"context_line":"                    subnet \u003d self._get_subnet(context,"},{"line_number":541,"context_line":"                                              original_ip[\u0027subnet_id\u0027])"},{"line_number":542,"context_line":"                    if (ipv6_utils.is_auto_address_subnet(subnet)):"}],"source_content_type":"text/x-python","patch_set":7,"id":"fa963961_c795d7e4","line":539,"in_reply_to":"1a6ced46_e3edcef0","updated":"2015-03-27 20:19:56.000000000","message":"We\u0027ll handle the SNAT update in a future patch, and remove the change from this patch.","commit_id":"9add7392301ec0c97941ac232dace477b44f5590"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"82738783f8f0b36c1f4548dcc1b015d207b4a093","unresolved":false,"context_lines":[{"line_number":453,"context_line":"                if not is_auto_addr:"},{"line_number":454,"context_line":"                    # Remove the IP address from the allocation pool"},{"line_number":455,"context_line":"                    NeutronDbPluginV2._allocate_specific_ip("},{"line_number":456,"context_line":"                        context, fixed[\u0027subnet_id\u0027], fixed[\u0027ip_address\u0027])"},{"line_number":457,"context_line":"                ips.append({\u0027ip_address\u0027: fixed[\u0027ip_address\u0027],"},{"line_number":458,"context_line":"                            \u0027subnet_id\u0027: fixed[\u0027subnet_id\u0027]})"},{"line_number":459,"context_line":"            # Only subnet ID is specified \u003d\u003e need to generate IP"}],"source_content_type":"text/x-python","patch_set":8,"id":"da9b358b_d854c355","line":456,"updated":"2015-03-29 17:01:15.000000000","message":"So what if the address passed isn\u0027t $prefix::1 but instead $prefix::EUI64 ?  I agree with the intent of the change but am worried someone will pass what the actual gateway address should be and mess this all up.  Or worse pass in some other EUI64 address they copy/pasted wrong.","commit_id":"04cd03840977bc32b2bcadcd185a8c4ae19b7159"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"eee4381d2f896878112d7ca2857d106a7ab04acd","unresolved":false,"context_lines":[{"line_number":453,"context_line":"                if not is_auto_addr:"},{"line_number":454,"context_line":"                    # Remove the IP address from the allocation pool"},{"line_number":455,"context_line":"                    NeutronDbPluginV2._allocate_specific_ip("},{"line_number":456,"context_line":"                        context, fixed[\u0027subnet_id\u0027], fixed[\u0027ip_address\u0027])"},{"line_number":457,"context_line":"                ips.append({\u0027ip_address\u0027: fixed[\u0027ip_address\u0027],"},{"line_number":458,"context_line":"                            \u0027subnet_id\u0027: fixed[\u0027subnet_id\u0027]})"},{"line_number":459,"context_line":"            # Only subnet ID is specified \u003d\u003e need to generate IP"}],"source_content_type":"text/x-python","patch_set":8,"id":"da9b358b_ea1d44be","line":456,"in_reply_to":"da9b358b_d49b4bc8","updated":"2015-03-30 20:44:09.000000000","message":"Dane, thanks for the explanation, and I do see now there is code in create_port() to store the IP in the DB, which is different from allocation, I just got tripped-up there.","commit_id":"04cd03840977bc32b2bcadcd185a8c4ae19b7159"},{"author":{"_account_id":6695,"name":"Dane LeBlanc","email":"leblancd@cisco.com","username":"leblancd"},"change_message_id":"79b990a541bb180e82b74c3db5f11facd84be1a4","unresolved":false,"context_lines":[{"line_number":453,"context_line":"                if not is_auto_addr:"},{"line_number":454,"context_line":"                    # Remove the IP address from the allocation pool"},{"line_number":455,"context_line":"                    NeutronDbPluginV2._allocate_specific_ip("},{"line_number":456,"context_line":"                        context, fixed[\u0027subnet_id\u0027], fixed[\u0027ip_address\u0027])"},{"line_number":457,"context_line":"                ips.append({\u0027ip_address\u0027: fixed[\u0027ip_address\u0027],"},{"line_number":458,"context_line":"                            \u0027subnet_id\u0027: fixed[\u0027subnet_id\u0027]})"},{"line_number":459,"context_line":"            # Only subnet ID is specified \u003d\u003e need to generate IP"}],"source_content_type":"text/x-python","patch_set":8,"id":"da9b358b_d49b4bc8","line":456,"in_reply_to":"da9b358b_d854c355","updated":"2015-03-30 19:28:30.000000000","message":"There is a uniqueness check that is performed in _test_fixed_ips_for_port() before we get to this point in _allocate_fixed_ips(). So if the user chooses an internal gateway IP address of the form $prefix::EUI64 and that address conflicts with a SLAAC address that is already in use on the subnet, then this will be detected and rejected. Similarly, if the user chooses an EUI64 address on the internal gateway port first, and then later on a VM instance is brought up that happens to use the same SLAAC address, then this will be detected via a uniqueness check in _allocate_ips_for_port(). Note that this change under review isn\u0027t changing this behavior from what is currently upstream. The change being proposed here is simply that the _allocate_specific_ip() should not get called when a user selects a specific IP on an internal router interface. The call to _allocate_specific_ip() with the current implementation doesn\u0027t do anything to prevent IP address conflicts between user-selected addresses and those assigned automatically via SLAAC.","commit_id":"04cd03840977bc32b2bcadcd185a8c4ae19b7159"}]}