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
