yahoo-eng-team team mailing list archive

[Carl Baldwin <carl@xxxxxxxxxxxxx>]

Public bug reported:

In looking through the IPAM code, I ran across this check [1] which is
as a major difference between the pluggable and non-pluggable IPAM code
paths.  I got to thinking about it, and I don't see a valid use case for
this check.

Let's say an ip_address was specified in fixed_ips and the subnet cidr
is detected by _get_subnet_for_fixed_ip as the provisional PD prefix
(::1/64).  That would mean that the IP address specified is contained in
::1/64 (i.e. ::1 or ::dead:beef:1:1) in order for check_subnet_ip  to
match it [2].  This doesn't make sense in the first place to specify
this kind of IP address for a subnet for which the network address isn't
even known.  We can't allocate such an IP to the port.

Now let's look at what happens if even if you do specify such a
nonsensical address.  This conditional is False and so it drops to the
else clause [3].  Here it is checked if it is a router port and if it is
an auto address subnet.  PD subnets are all auto address subnets, so it
depends on if it is a router port.  If it is, the  subnet is added to
the IPs to allocate *without* the ip address!  If  it isn't, the entire
thing is ignored.  Either way, the specific IP address that was
specified is ignored.  All auto address subnets are added to the list
later so it doesn't even matter that it *might* have been added here.

So, if I understand everything correctly, this check doesn't make sense.

[1] https://github.com/openstack/neutron/blob/c5bc5bda34/neutron/db/ipam_non_pluggable_backend.py#L248-249
[2]
https://github.com/openstack/neutron/blob/c5bc5bda34/neutron/ipam/utils.py#L28
[3] https://github.com/openstack/neutron/blob/c5bc5bda34/neutron/db/ipam_non_pluggable_backend.py#L268

** Affects: neutron
     Importance: Low
     Assignee: Carl Baldwin (carl-baldwin)
         Status: In Progress


** Tags: l3-ipam-dhcp

** Changed in: neutron
   Importance: Undecided => Low

** Tags added: l3-ipam-dhcp

-- 
You received this bug notification because you are a member of Yahoo!
Engineering Team, which is subscribed to neutron.
https://bugs.launchpad.net/bugs/1568992

Title:
  PD adds a non-sensical check to nonpluggable IPAM

Status in neutron:
  In Progress

Bug description:
  In looking through the IPAM code, I ran across this check [1] which is
  as a major difference between the pluggable and non-pluggable IPAM
  code paths.  I got to thinking about it, and I don't see a valid use
  case for this check.

  Let's say an ip_address was specified in fixed_ips and the subnet cidr
  is detected by _get_subnet_for_fixed_ip as the provisional PD prefix
  (::1/64).  That would mean that the IP address specified is contained
  in ::1/64 (i.e. ::1 or ::dead:beef:1:1) in order for check_subnet_ip
  to match it [2].  This doesn't make sense in the first place to
  specify this kind of IP address for a subnet for which the network
  address isn't even known.  We can't allocate such an IP to the port.

  Now let's look at what happens if even if you do specify such a
  nonsensical address.  This conditional is False and so it drops to the
  else clause [3].  Here it is checked if it is a router port and if it
  is an auto address subnet.  PD subnets are all auto address subnets,
  so it depends on if it is a router port.  If it is, the  subnet is
  added to the IPs to allocate *without* the ip address!  If  it isn't,
  the entire thing is ignored.  Either way, the specific IP address that
  was specified is ignored.  All auto address subnets are added to the
  list later so it doesn't even matter that it *might* have been added
  here.

  So, if I understand everything correctly, this check doesn't make
  sense.

  [1] https://github.com/openstack/neutron/blob/c5bc5bda34/neutron/db/ipam_non_pluggable_backend.py#L248-249
  [2]
  https://github.com/openstack/neutron/blob/c5bc5bda34/neutron/ipam/utils.py#L28
  [3] https://github.com/openstack/neutron/blob/c5bc5bda34/neutron/db/ipam_non_pluggable_backend.py#L268

To manage notifications about this bug go to:
https://bugs.launchpad.net/neutron/+bug/1568992/+subscriptions