openwisp / openwisp-ipam

IP address space administration module of OpenWISP
https://openwisp.io/docs/dev/ipam/
BSD 3-Clause "New" or "Revised" License
104 stars 51 forks source link

[bug] Subnet /32 gives error in pie chart #129

Closed nemesifier closed 2 years ago

nemesifier commented 2 years ago

Try creating a /32 subnet.

Expected result:

Actual result:

Screenshot from 2022-01-05 11-38-52

Aryamanz29 commented 2 years ago

As seen from the above screenshot, Endpoint

http://127.0.0.1:8000/api/v1/ipam/subnet/64d5dfc7-7845-42c1-8856-d3436dddc750/hosts/

Causes "ValueError __len__() should return >= 0", I'm wondering why broadcast == self.network in /32 subnet 🤔?, Also I tried return broadcast which solves pie chart problem, But I think that's not the correct way to solve this 😅 @nemesisdesign

class HostsSet:
......
......
def count(self):
   # IPV4 (exclude broadcast)
   if self.subnet.subnet.max_prefixlen == 32:
      return broadcast - self.network - 1 #returns -1
......
......

Screenshot from 2022-02-05 23-40-37

pandafy commented 2 years ago

After discussing with @Aryamanz29 during sprint hours, we found out that updating code as following fixes this issues.

diff --git a/openwisp_ipam/api/views.py b/openwisp_ipam/api/views.py
index acae174..d918a49 100644
--- a/openwisp_ipam/api/views.py
+++ b/openwisp_ipam/api/views.py
@@ -138,9 +138,8 @@ class HostsSet:
             return self.stop - self.start
         broadcast = int(self.subnet.subnet.broadcast_address)
-        # IPV4 (exclude broadcast)
-        if self.subnet.subnet.max_prefixlen == 32:
-            return broadcast - self.network - 1
-        # IPV6
+        if self.subnet.subnet.max_prefixlen == 32 or self.subnet.subnet.max_prefixlen == 128:
+            return 1
         else:
             return broadcast - self.network

In my opinion, the comments in above code were misleading.

Aryamanz29 commented 2 years ago

After discussing with @Aryamanz29 during sprint hours, we found out that updating code as following fixes this issues.

diff --git a/openwisp_ipam/api/views.py b/openwisp_ipam/api/views.py
index acae174..d918a49 100644
--- a/openwisp_ipam/api/views.py
+++ b/openwisp_ipam/api/views.py
@@ -138,9 +138,8 @@ class HostsSet:
             return self.stop - self.start
         broadcast = int(self.subnet.subnet.broadcast_address)
-        # IPV4 (exclude broadcast)
-        if self.subnet.subnet.max_prefixlen == 32:
-            return broadcast - self.network - 1
-        # IPV6
+        if self.subnet.subnet.max_prefixlen == 32 or self.subnet.subnet.max_prefixlen == 128:
+            return 1
         else:
             return broadcast - self.network

In my opinion, the comments in above code were misleading.

(we can use _prefixlen for subnet /32),

 if self.subnet.subnet.max_prefixlen == 32:
            if self.subnet.subnet._prefixlen == 32:
                return 1
            return broadcast - self.network - 1
        else:
            return broadcast - self.network

(Only for subnet /32, it returns single host that's why we encountered this bug , See docs https://docs.python.org/3/library/ipaddress.html#ipaddress.IPv4Network.hosts

My findings :

from ipaddress import ip_network

print('_prefixlen :')
print(ip_network('192.0.0.0/16')._prefixlen)
print(ip_network('192.0.0.0/29')._prefixlen)
print(ip_network('192.0.0.0/32')._prefixlen)
print(ip_network('fdb6:21b:a477::/64')._prefixlen)

print('prefixlen :')
print(ip_network('192.0.0.0/16').max_prefixlen)
print(ip_network('192.0.0.0/29').max_prefixlen)
print(ip_network('192.0.0.0/32').max_prefixlen)
print(ip_network('fdb6:21b:a477::/64').max_prefixlen)

print('192.0.2.0/29')
print('HOSTS : ',list(ip_network('192.0.2.0/29').hosts()))

print()
# Our case
print('Our case :', '192.0.0.0/32')
print('HOSTS : ',list(ip_network('192.0.0.0/32').hosts()))
(env) ➜  python3 subnet.py
_prefixlen :
16
29
32
64
prefixlen :
32
32
32
128
192.0.2.0/29
HOSTS :  [IPv4Address('192.0.2.1'), IPv4Address('192.0.2.2'), IPv4Address('192.0.2.3'), IPv4Address('192.0.2.4'), IPv4Address('192.0.2.5'), IPv4Address('192.0.2.6')]

Our case : 192.0.0.0/32
HOSTS :  [IPv4Address('192.0.0.0')]

NOW :

Screenshot from 2022-02-22 23-22-50