Re: [openstack-dev] [neutron] Neutron should disallow /32 CIDR
I think I agree. The new check isn't adding much value and we could debate for a long time whether /30 is useful and should be disallowed or not. There are bigger fish to fry. Carl On Fri, Jan 24, 2014 at 10:43 AM, Paul Ward wpw...@us.ibm.com wrote: Given your obviously much more extensive understanding of networking than mine, I'm starting to move over to the we shouldn't make this fix camp. Mostly because of this: CARVER, PAUL pc2...@att.com wrote on 01/23/2014 08:57:10 PM: Putting a friendly helper in Horizon will help novice users and provide a good example to anyone who is developing an alternate UI to invoke the Neutron API. I’m not sure what the benefit is of putting code in the backend to disallow valid but silly subnet masks. I include /30, /31, AND /32 in the category of “silly” subnet masks to use on a broadcast medium. All three are entirely legitimate subnet masks, it’s just that they’re not useful for end host networks. My mindset has always been that we should programmatically prevent things that are definitively wrong. Of which, these netmasks apparently are not. So it would seem we should leave neutron server code alone under the assumption that those using CLI to create networks *probably* know what they're doing. However, the UI is supposed to be the more friendly interface and perhaps this is the more appropriate place for this change? As I stated before, horizon prevents /32, but allows /31. I'm no UI guy, so maybe the best course of action is to abandon my change in gerrit and move the launchpad bug back to unassigned and see if someone with horizon experience wants to pick this up. What do others think about this? Thanks again for your participation in this discussion, Paul. It's been very enlightening to me. ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [neutron] Neutron should disallow /32 CIDR
Well say Carl! I am sorry I did not get back to you on this topic before. In general and after thinking about it, it makes sense to leave could admin to adage the /32 cases if any. Edgar On 1/28/14 1:46 PM, Carl Baldwin c...@ecbaldwin.net wrote: I think I agree. The new check isn't adding much value and we could debate for a long time whether /30 is useful and should be disallowed or not. There are bigger fish to fry. Carl On Fri, Jan 24, 2014 at 10:43 AM, Paul Ward wpw...@us.ibm.com wrote: Given your obviously much more extensive understanding of networking than mine, I'm starting to move over to the we shouldn't make this fix camp. Mostly because of this: CARVER, PAUL pc2...@att.com wrote on 01/23/2014 08:57:10 PM: Putting a friendly helper in Horizon will help novice users and provide a good example to anyone who is developing an alternate UI to invoke the Neutron API. I¹m not sure what the benefit is of putting code in the backend to disallow valid but silly subnet masks. I include /30, /31, AND /32 in the category of ³silly² subnet masks to use on a broadcast medium. All three are entirely legitimate subnet masks, it¹s just that they¹re not useful for end host networks. My mindset has always been that we should programmatically prevent things that are definitively wrong. Of which, these netmasks apparently are not. So it would seem we should leave neutron server code alone under the assumption that those using CLI to create networks *probably* know what they're doing. However, the UI is supposed to be the more friendly interface and perhaps this is the more appropriate place for this change? As I stated before, horizon prevents /32, but allows /31. I'm no UI guy, so maybe the best course of action is to abandon my change in gerrit and move the launchpad bug back to unassigned and see if someone with horizon experience wants to pick this up. What do others think about this? Thanks again for your participation in this discussion, Paul. It's been very enlightening to me. ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [neutron] Neutron should disallow /32 CIDR
Given your obviously much more extensive understanding of networking than mine, I'm starting to move over to the we shouldn't make this fix camp. Mostly because of this: CARVER, PAUL pc2...@att.com wrote on 01/23/2014 08:57:10 PM: Putting a friendly helper in Horizon will help novice users and provide a good example to anyone who is developing an alternate UI to invoke the Neutron API. I’m not sure what the benefit is of putting code in the backend to disallow valid but silly subnet masks. I include /30, /31, AND /32 in the category of “silly” subnet masks to use on a broadcast medium. All three are entirely legitimate subnet masks, it’s just that they’re not useful for end host networks. My mindset has always been that we should programmatically prevent things that are definitively wrong. Of which, these netmasks apparently are not. So it would seem we should leave neutron server code alone under the assumption that those using CLI to create networks *probably* know what they're doing. However, the UI is supposed to be the more friendly interface and perhaps this is the more appropriate place for this change? As I stated before, horizon prevents /32, but allows /31. I'm no UI guy, so maybe the best course of action is to abandon my change in gerrit and move the launchpad bug back to unassigned and see if someone with horizon experience wants to pick this up. What do others think about this? Thanks again for your participation in this discussion, Paul. It's been very enlightening to me.___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [neutron] Neutron should disallow /32 CIDR
Paul Ward: Thank you to all who have participated in this thread. I've just proposed a fix in gerrit. For those involved thus far, if you could review I would be greatly appreciative! https://review.openstack.org/#/c/68742/1 I wouldn't go so far as to say this verification SHOULDN'T be added, but neither would I say it should. From a general use case perspective I don't think IPv4 subnets smaller than /29 make sense. A /32 is a commonly used subnet length for some use cases (e.g. router loopback interface) but may not have an applicable use in a cloud network. I have never seen a /31 network used anywhere. Point to point links (e.g. T1/Frame Relay/etc) are often /30 but I've never seen a /30 subnet for anything other than connecting two routers. However, does it really benefit the user to specifically block them from entering /32 or block them from entering /30, /31, and /32? It might not be an equal amount of code, I think a much better effort to help the user would be to provide them with a subnet calculator directly in Horizon to show them how many usable IPs are in the subnet they're defining. In this case, displaying Usable addresses: 0 right when they enter /32 would be helpful and they would figure out for themselves whether they really wanted that mask or if they meant something else? ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [neutron] Neutron should disallow /32 CIDR
Paul Ward wrote: Given your statement about routers potentially using a /30 network, I think we should leave the restriction at /30 rather than /29. I'm assuming your statement that some routers use /30 subnets to connect to each other could potentially apply to neutron-created routers. Generally speaking I’ve only seen /30 on point to point links. For most purposes the network number (all zeros host part) and broadcast (all ones host part) are not valid host addresses so a /30 only has two usable addresses. This is perfect on a point to point link which only has two endpoints, but kind of silly on a broadcast domain. However, what is the user perceived impact of your code? Will the error message propagate back to the user directly or do they simply get an unexplained failure while the explanatory error message gets dumped into a log file which is visible only to the provider, not the tenant? Putting a friendly helper in Horizon will help novice users and provide a good example to anyone who is developing an alternate UI to invoke the Neutron API. I’m not sure what the benefit is of putting code in the backend to disallow valid but silly subnet masks. I include /30, /31, AND /32 in the category of “silly” subnet masks to use on a broadcast medium. All three are entirely legitimate subnet masks, it’s just that they’re not useful for end host networks. My reasoning behind checking the number of IP addresses in the subnet rather than the actual CIDR prefix length is that I want the code to be IP version agnostic. If we're talking IPv6, then /30 isn't going to be relevant. I'm not overly familiar with IPv6, but is it safe to say it has the same restriction that there must be more than 2 IPs available as the highest IP is the broadcast? No, it actually isn’t safe to say that at all. IPv6, in what I consider to be “broken by design” but others defend vehemently, mandates that no subnet mask longer (i.e. numerically larger) than /64 is allowed. Some people grudgingly acknowledge that a /128 mask is acceptable for router loopback addresses and fewer people accept /126 for point to point links, but pretty much everyone agrees that anything between /64 to /126 should never be used. Some people state that using a mask longer than /64 will break fundamental parts IPv6. I’m of the opinion that it was already broken and using masks longer than /64 merely reveals what’s broken rather than causing the breakage. Nevertheless, it’s too late to change those parts of IPv6 now. They’re set in stone and we’re stuck with the fact that all broadcast domains (i.e. layer 2 segments, VLANs, end host subnets) must be /64 and nothing else. Masks shorter than /64 (i.e. numbers smaller than 64) are for route aggregation and belong in the world of routing protocols and IPAM databases. One could probably make a convincing argument that as far as neutron subnet-create is concerned there’s no reason to even accept a subnet mask at all. Anything other than /64 is pretty much 99.999% guaranteed to be user error. ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [neutron] Neutron should disallow /32 CIDR
Thanks for your input, Carl. You're right, it seems the more appropriate place for this is _validate_subnet(). It checks ip version, gateway, etc... but not the size of the subnet. Carl Baldwin c...@ecbaldwin.net wrote on 01/21/2014 09:22:55 PM: From: Carl Baldwin c...@ecbaldwin.net To: OpenStack Development Mailing List openstack-dev@lists.openstack.org, Date: 01/21/2014 09:27 PM Subject: Re: [openstack-dev] [neutron] Neutron should disallow /32 CIDR The bottom line is that the method you mentioned shouldn't validate the subnet. It should assume the subnet has been validated and validate the pool. It seems to do a adequate job of that. Perhaps there is a _validate_subnet method that you should be focused on? (I'd check but I don't have convenient access to the code at the moment) Carl On Jan 21, 2014 6:16 PM, Paul Ward wpw...@us.ibm.com wrote: You beat me to it. :) I just responded about not checking the allocation pool start and end but rather, checking subnet_first_ip and subnet_last_ip, which is set as follows: subnet = netaddr.IPNetwork(subnet_cidr) subnet_first_ip = netaddr.IPAddress(subnet.first + 1) subnet_last_ip = netaddr.IPAddress(subnet.last - 1) However, I'm curious about your contention that we're ok... I'm assuming you mean that this should already be handled. I don't believe anything is really checking to be sure the allocation pool leaves room for a gateway, I think it just makes sure it fits in the subnet. A member of our test team successfully created a network with a subnet of 255.255.255.255, so it got through somehow. I will look into that more tomorrow. Carl Baldwin c...@ecbaldwin.net wrote on 01/21/2014 05:27:49 PM: From: Carl Baldwin c...@ecbaldwin.net To: OpenStack Development Mailing List (not for usage questions) openstack-dev@lists.openstack.org, Date: 01/21/2014 05:32 PM Subject: Re: [openstack-dev] [neutron] Neutron should disallow /32 CIDR I think there may be some confusion between the two concepts: subnet and allocation pool. You are right that an ipv4 subnet smaller than /30 is not useable on a network. However, this method is checking the validity of an allocation pool. These pools should not include room for a gateway nor broadcast address. Their relation to subnets is that the range of ips contained in the pool must fit within the allocatable IP space on the subnet from which they are allocated. Other than that, they are simple ranges; they don't need to be cidr aligned or anything. A pool of a single IP is valid. I just checked the method's implementation now. It does check that the pool fits within the allocatable range of the subnet. I think we're good. Carl On Tue, Jan 21, 2014 at 3:35 PM, Paul Ward wpw...@us.ibm.com wrote: Currently, NeutronDbPluginV2._validate_allocation_pools() does some very basic checking to be sure the specified subnet is valid. One thing that's missing is checking for a CIDR of /32. A subnet with one IP address in it is unusable as the sole IP address will be allocated to the gateway, and thus no IPs are left over to be allocated to VMs. The fix for this is simple. In NeutronDbPluginV2._validate_allocation_pools(), we'd check for start_ip == end_ip and raise an exception if that's true. I've opened lauchpad bug report 1271311 (https://bugs.launchpad.net/neutron/+bug/1271311) for this, but wanted to start a discussion here to see if others find this enhancement to be a valuable addition. ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [neutron] Neutron should disallow /32 CIDR
Agreed. That would be a good place for that check. Carl On Wed, Jan 22, 2014 at 6:40 AM, Paul Ward wpw...@us.ibm.com wrote: Thanks for your input, Carl. You're right, it seems the more appropriate place for this is _validate_subnet(). It checks ip version, gateway, etc... but not the size of the subnet. Carl Baldwin c...@ecbaldwin.net wrote on 01/21/2014 09:22:55 PM: From: Carl Baldwin c...@ecbaldwin.net To: OpenStack Development Mailing List openstack-dev@lists.openstack.org, Date: 01/21/2014 09:27 PM Subject: Re: [openstack-dev] [neutron] Neutron should disallow /32 CIDR The bottom line is that the method you mentioned shouldn't validate the subnet. It should assume the subnet has been validated and validate the pool. It seems to do a adequate job of that. Perhaps there is a _validate_subnet method that you should be focused on? (I'd check but I don't have convenient access to the code at the moment) Carl On Jan 21, 2014 6:16 PM, Paul Ward wpw...@us.ibm.com wrote: You beat me to it. :) I just responded about not checking the allocation pool start and end but rather, checking subnet_first_ip and subnet_last_ip, which is set as follows: subnet = netaddr.IPNetwork(subnet_cidr) subnet_first_ip = netaddr.IPAddress(subnet.first + 1) subnet_last_ip = netaddr.IPAddress(subnet.last - 1) However, I'm curious about your contention that we're ok... I'm assuming you mean that this should already be handled. I don't believe anything is really checking to be sure the allocation pool leaves room for a gateway, I think it just makes sure it fits in the subnet. A member of our test team successfully created a network with a subnet of 255.255.255.255, so it got through somehow. I will look into that more tomorrow. Carl Baldwin c...@ecbaldwin.net wrote on 01/21/2014 05:27:49 PM: From: Carl Baldwin c...@ecbaldwin.net To: OpenStack Development Mailing List (not for usage questions) openstack-dev@lists.openstack.org, Date: 01/21/2014 05:32 PM Subject: Re: [openstack-dev] [neutron] Neutron should disallow /32 CIDR I think there may be some confusion between the two concepts: subnet and allocation pool. You are right that an ipv4 subnet smaller than /30 is not useable on a network. However, this method is checking the validity of an allocation pool. These pools should not include room for a gateway nor broadcast address. Their relation to subnets is that the range of ips contained in the pool must fit within the allocatable IP space on the subnet from which they are allocated. Other than that, they are simple ranges; they don't need to be cidr aligned or anything. A pool of a single IP is valid. I just checked the method's implementation now. It does check that the pool fits within the allocatable range of the subnet. I think we're good. Carl On Tue, Jan 21, 2014 at 3:35 PM, Paul Ward wpw...@us.ibm.com wrote: Currently, NeutronDbPluginV2._validate_allocation_pools() does some very basic checking to be sure the specified subnet is valid. One thing that's missing is checking for a CIDR of /32. A subnet with one IP address in it is unusable as the sole IP address will be allocated to the gateway, and thus no IPs are left over to be allocated to VMs. The fix for this is simple. In NeutronDbPluginV2._validate_allocation_pools(), we'd check for start_ip == end_ip and raise an exception if that's true. I've opened lauchpad bug report 1271311 (https://bugs.launchpad.net/neutron/+bug/1271311) for this, but wanted to start a discussion here to see if others find this enhancement to be a valuable addition. ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [neutron] Neutron should disallow /32 CIDR
Wouldn't be easier just to check if: cidr is 32? I believe it is a good idea to not allow /32 network but this is just my opinion Edgar From: Paul Ward wpw...@us.ibm.com Reply-To: OpenStack List openstack-dev@lists.openstack.org Date: Tuesday, January 21, 2014 12:35 PM To: OpenStack List openstack-dev@lists.openstack.org Subject: [openstack-dev] [neutron] Neutron should disallow /32 CIDR Currently, NeutronDbPluginV2._validate_allocation_pools() does some very basic checking to be sure the specified subnet is valid. One thing that's missing is checking for a CIDR of /32. A subnet with one IP address in it is unusable as the sole IP address will be allocated to the gateway, and thus no IPs are left over to be allocated to VMs. The fix for this is simple. In NeutronDbPluginV2._validate_allocation_pools(), we'd check for start_ip == end_ip and raise an exception if that's true. I've opened lauchpad bug report 1271311 (https://bugs.launchpad.net/neutron/+bug/1271311) for this, but wanted to start a discussion here to see if others find this enhancement to be a valuable addition. ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [neutron] Neutron should disallow /32 CIDR
/30 is the minimum allowable mask, not /31. On 21 January 2014 22:04, Edgar Magana emag...@plumgrid.com wrote: Wouldn't be easier just to check if: cidr is 32? I believe it is a good idea to not allow /32 network but this is just my opinion Edgar From: Paul Ward wpw...@us.ibm.com Reply-To: OpenStack List openstack-dev@lists.openstack.org Date: Tuesday, January 21, 2014 12:35 PM To: OpenStack List openstack-dev@lists.openstack.org Subject: [openstack-dev] [neutron] Neutron should disallow /32 CIDR Currently, NeutronDbPluginV2._validate_allocation_pools() does some very basic checking to be sure the specified subnet is valid. One thing that's missing is checking for a CIDR of /32. A subnet with one IP address in it is unusable as the sole IP address will be allocated to the gateway, and thus no IPs are left over to be allocated to VMs. The fix for this is simple. In NeutronDbPluginV2._validate_allocation_pools(), we'd check for start_ip == end_ip and raise an exception if that's true. I've opened lauchpad bug report 1271311 ( https://bugs.launchpad.net/neutron/+bug/1271311) for this, but wanted to start a discussion here to see if others find this enhancement to be a valuable addition. ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [neutron] Neutron should disallow /32 CIDR
Using a /32 has use cases. When trying to build a L3 only routed network /32 routes are required. While it can be inconvenient for the more generic L2 network use case, I wouldn't remove it since there are other use cases where it is useful. -Carl On 01/21/2014 12:35 PM, Paul Ward wrote: Currently, NeutronDbPluginV2._validate_allocation_pools() does some very basic checking to be sure the specified subnet is valid. One thing that's missing is checking for a CIDR of /32. A subnet with one IP address in it is unusable as the sole IP address will be allocated to the gateway, and thus no IPs are left over to be allocated to VMs. The fix for this is simple. In NeutronDbPluginV2._validate_allocation_pools(), we'd check for start_ip == end_ip and raise an exception if that's true. I've opened lauchpad bug report 1271311 (https://bugs.launchpad.net/neutron/+bug/1271311) for this, but wanted to start a discussion here to see if others find this enhancement to be a valuable addition. ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev signature.asc Description: OpenPGP digital signature ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [neutron] Neutron should disallow /32 CIDR
I think there may be some confusion between the two concepts: subnet and allocation pool. You are right that an ipv4 subnet smaller than /30 is not useable on a network. However, this method is checking the validity of an allocation pool. These pools should not include room for a gateway nor broadcast address. Their relation to subnets is that the range of ips contained in the pool must fit within the allocatable IP space on the subnet from which they are allocated. Other than that, they are simple ranges; they don't need to be cidr aligned or anything. A pool of a single IP is valid. I just checked the method's implementation now. It does check that the pool fits within the allocatable range of the subnet. I think we're good. Carl On Tue, Jan 21, 2014 at 3:35 PM, Paul Ward wpw...@us.ibm.com wrote: Currently, NeutronDbPluginV2._validate_allocation_pools() does some very basic checking to be sure the specified subnet is valid. One thing that's missing is checking for a CIDR of /32. A subnet with one IP address in it is unusable as the sole IP address will be allocated to the gateway, and thus no IPs are left over to be allocated to VMs. The fix for this is simple. In NeutronDbPluginV2._validate_allocation_pools(), we'd check for start_ip == end_ip and raise an exception if that's true. I've opened lauchpad bug report 1271311 (https://bugs.launchpad.net/neutron/+bug/1271311) for this, but wanted to start a discussion here to see if others find this enhancement to be a valuable addition. ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [neutron] Neutron should disallow /32 CIDR
Possibly, though I don't see code that checks the actual CIDR length. It seems to check CIDR correctness via IP correctness. ie, things like the ending IP not being smaller than the starting IP, etc. One change to my original message on what the fix is, we'd have to compare subnet_first_ip and subnet_last_ip... not start_ip and end_ip as those are from the pool passed in, not the actual first and last IPs in the subnet. In the launchpad bug report, it was mentioned you can create a subnet without a gateway. I would still contend this is invalid because then you have a VM on a single-IP subnet without a gateway, which is also a dead end. Thoughts? Edgar Magana emag...@plumgrid.com wrote on 01/21/2014 03:04:47 PM: From: Edgar Magana emag...@plumgrid.com To: OpenStack List openstack-dev@lists.openstack.org, Date: 01/21/2014 03:10 PM Subject: Re: [openstack-dev] [neutron] Neutron should disallow /32 CIDR Wouldn't be easier just to check if: cidr is 32? I believe it is a good idea to not allow /32 network but this is just my opinion Edgar From: Paul Ward wpw...@us.ibm.com Reply-To: OpenStack List openstack-dev@lists.openstack.org Date: Tuesday, January 21, 2014 12:35 PM To: OpenStack List openstack-dev@lists.openstack.org Subject: [openstack-dev] [neutron] Neutron should disallow /32 CIDR Currently, NeutronDbPluginV2._validate_allocation_pools() does some very basic checking to be sure the specified subnet is valid. One thing that's missing is checking for a CIDR of /32. A subnet with one IP address in it is unusable as the sole IP address will be allocated to the gateway, and thus no IPs are left over to be allocated to VMs. The fix for this is simple. In NeutronDbPluginV2._validate_allocation_pools(), we'd check for start_ip == end_ip and raise an exception if that's true. I've opened lauchpad bug report 1271311 (https://bugs.launchpad.net/ neutron/+bug/1271311) for this, but wanted to start a discussion here to see if others find this enhancement to be a valuable addition. ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi- bin/mailman/listinfo/openstack-dev ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [neutron] Neutron should disallow /32 CIDR
You beat me to it. :) I just responded about not checking the allocation pool start and end but rather, checking subnet_first_ip and subnet_last_ip, which is set as follows: subnet = netaddr.IPNetwork(subnet_cidr) subnet_first_ip = netaddr.IPAddress(subnet.first + 1) subnet_last_ip = netaddr.IPAddress(subnet.last - 1) However, I'm curious about your contention that we're ok... I'm assuming you mean that this should already be handled. I don't believe anything is really checking to be sure the allocation pool leaves room for a gateway, I think it just makes sure it fits in the subnet. A member of our test team successfully created a network with a subnet of 255.255.255.255, so it got through somehow. I will look into that more tomorrow. Carl Baldwin c...@ecbaldwin.net wrote on 01/21/2014 05:27:49 PM: From: Carl Baldwin c...@ecbaldwin.net To: OpenStack Development Mailing List (not for usage questions) openstack-dev@lists.openstack.org, Date: 01/21/2014 05:32 PM Subject: Re: [openstack-dev] [neutron] Neutron should disallow /32 CIDR I think there may be some confusion between the two concepts: subnet and allocation pool. You are right that an ipv4 subnet smaller than /30 is not useable on a network. However, this method is checking the validity of an allocation pool. These pools should not include room for a gateway nor broadcast address. Their relation to subnets is that the range of ips contained in the pool must fit within the allocatable IP space on the subnet from which they are allocated. Other than that, they are simple ranges; they don't need to be cidr aligned or anything. A pool of a single IP is valid. I just checked the method's implementation now. It does check that the pool fits within the allocatable range of the subnet. I think we're good. Carl On Tue, Jan 21, 2014 at 3:35 PM, Paul Ward wpw...@us.ibm.com wrote: Currently, NeutronDbPluginV2._validate_allocation_pools() does some very basic checking to be sure the specified subnet is valid. One thing that's missing is checking for a CIDR of /32. A subnet with one IP address in it is unusable as the sole IP address will be allocated to the gateway, and thus no IPs are left over to be allocated to VMs. The fix for this is simple. In NeutronDbPluginV2._validate_allocation_pools(), we'd check for start_ip == end_ip and raise an exception if that's true. I've opened lauchpad bug report 1271311 (https://bugs.launchpad.net/neutron/+bug/1271311) for this, but wanted to start a discussion here to see if others find this enhancement to be a valuable addition. ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [neutron] Neutron should disallow /32 CIDR
The bottom line is that the method you mentioned shouldn't validate the subnet. It should assume the subnet has been validated and validate the pool. It seems to do a adequate job of that. Perhaps there is a _validate_subnet method that you should be focused on? (I'd check but I don't have convenient access to the code at the moment) Carl On Jan 21, 2014 6:16 PM, Paul Ward wpw...@us.ibm.com wrote: You beat me to it. :) I just responded about not checking the allocation pool start and end but rather, checking subnet_first_ip and subnet_last_ip, which is set as follows: subnet *=* netaddr*.*IPNetwork(subnet_cidr) subnet_first_ip *=* netaddr*.*IPAddress(subnet*.*first *+* 1) subnet_last_ip *=* netaddr*.*IPAddress(subnet*.*last *-* 1) However, I'm curious about your contention that we're ok... I'm assuming you mean that this should already be handled. I don't believe anything is really checking to be sure the allocation pool leaves room for a gateway, I think it just makes sure it fits in the subnet. A member of our test team successfully created a network with a subnet of 255.255.255.255, so it got through somehow. I will look into that more tomorrow. Carl Baldwin c...@ecbaldwin.net wrote on 01/21/2014 05:27:49 PM: From: Carl Baldwin c...@ecbaldwin.net To: OpenStack Development Mailing List (not for usage questions) openstack-dev@lists.openstack.org, Date: 01/21/2014 05:32 PM Subject: Re: [openstack-dev] [neutron] Neutron should disallow /32 CIDR I think there may be some confusion between the two concepts: subnet and allocation pool. You are right that an ipv4 subnet smaller than /30 is not useable on a network. However, this method is checking the validity of an allocation pool. These pools should not include room for a gateway nor broadcast address. Their relation to subnets is that the range of ips contained in the pool must fit within the allocatable IP space on the subnet from which they are allocated. Other than that, they are simple ranges; they don't need to be cidr aligned or anything. A pool of a single IP is valid. I just checked the method's implementation now. It does check that the pool fits within the allocatable range of the subnet. I think we're good. Carl On Tue, Jan 21, 2014 at 3:35 PM, Paul Ward wpw...@us.ibm.com wrote: Currently, NeutronDbPluginV2._validate_allocation_pools() does some very basic checking to be sure the specified subnet is valid. One thing that's missing is checking for a CIDR of /32. A subnet with one IP address in it is unusable as the sole IP address will be allocated to the gateway, and thus no IPs are left over to be allocated to VMs. The fix for this is simple. In NeutronDbPluginV2._validate_allocation_pools(), we'd check for start_ip == end_ip and raise an exception if that's true. I've opened lauchpad bug report 1271311 (https://bugs.launchpad.net/neutron/+bug/1271311) for this, but wanted to start a discussion here to see if others find this enhancement to be a valuable addition. ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
