Cidr4 merge algorithm #5

Merged
PavelPatsey merged 91 commits from CIDR4_merge_algorithm into main 2025-01-27 22:05:39 +03:00
Showing only changes of commit f7156fb61d - Show all commits
+19 -29
View File
@@ -1,45 +1,35 @@
from copy import deepcopy import ipaddress
Fedor-Lyanguzov commented 2025-01-07 17:54:03 +03:00 (Migrated from github.com)
Review

Здесь за один проход объединяются все возможные объединения, таким образом мы можем промахнуться мимо цели в M элементов списка.

Здесь за один проход объединяются все возможные объединения, таким образом мы можем промахнуться мимо цели в `M` элементов списка.
Fedor-Lyanguzov commented 2025-01-07 17:55:32 +03:00 (Migrated from github.com)
Review

Здесь не хватает возвращения количества адресов, "попавших под раздачу": не принадлежащих начальному списку, но попавших в результат из-за объединения. Это количество позволит найти оптимальное решение.

Здесь не хватает возвращения количества адресов, "попавших под раздачу": не принадлежащих начальному списку, но попавших в результат из-за объединения. Это количество позволит найти оптимальное решение.
Fedor-Lyanguzov commented 2025-01-07 17:57:45 +03:00 (Migrated from github.com)
Review

Я думаю, использование внешней библиотеки (и вообще любой библиотеки) размывает смысл алгоритма: хотя из следующих строк понятно, что будет сделано; однако не очевидно, как это будет сделано, и будет ли оптимальный ответ, и будет ли оптимальное решение (что не обязательно).

Я думаю, использование внешней библиотеки (и вообще любой библиотеки) размывает смысл алгоритма: хотя из следующих строк понятно, что будет сделано; однако не очевидно, как это будет сделано, и будет ли оптимальный ответ, и будет ли оптимальное решение (что не обязательно).
Fedor-Lyanguzov commented 2025-01-12 17:46:01 +03:00 (Migrated from github.com)
Review

Мне кажется, что функция merge_nodes содержит не все свои обязанности, некоторые из них похоже вложись в функцию reduce_nodes. Стоит их переместить.

Мне кажется, что функция `merge_nodes` содержит не все свои обязанности, некоторые из них похоже вложись в функцию `reduce_nodes`. Стоит их переместить.
Fedor-Lyanguzov commented 2025-01-12 19:00:28 +03:00 (Migrated from github.com)
Review

Алгоритм слишком много делает каждый шаг, из-за этого работает медленно. Как мне кажется, для оптимизации стоит разработать алгоритм начиная с рекурсии, возможно их будет штук 5 связанных, зато это позволит определить характеристики отдельных кусков и принять решение по оптимизации.

Алгоритм слишком много делает каждый шаг, из-за этого работает медленно. Как мне кажется, для оптимизации стоит разработать алгоритм начиная с рекурсии, возможно их будет штук 5 связанных, зато это позволит определить характеристики отдельных кусков и принять решение по оптимизации.
Fedor-Lyanguzov commented 2025-01-12 19:01:34 +03:00 (Migrated from github.com)
Review

Пора перенести тесты в отдельный файл?

Пора перенести тесты в отдельный файл?
from typing import List from ipaddress import IPv4Address
from typing import List, Tuple
from netaddr import IPNetwork, cidr_merge
def get_ips(input_file) -> List[IPNetwork]: def get_data(input_file):
with open(input_file, "r") as file: with open(input_file, "r") as file:
data = file.read().splitlines() data = file.read().splitlines()
return [IPNetwork(line) for line in data] return data
def reduce_prefixlen(ip: IPNetwork, step=1) -> IPNetwork: def cidr4_to_binary(cidr4: str) -> Tuple[str, int]:
"""Reduce the CIDR prefix len by step""" ip_str, vlsm = cidr4.strip().split("/")
new_ip = IPNetwork(ip) vlsm = int(vlsm)
new_ip.prefixlen -= step ipv4 = IPv4Address(ip_str)
return IPNetwork(new_ip.cidr) binary_ip = bin(int(ipv4))[2:]
return binary_ip, vlsm
def merge_ips(ips_to_reduce: List[IPNetwork], max_len, step=1) -> List[IPNetwork]: def binary_to_cidr4(binary_ip: str, vlsm: int) -> str:
ips = deepcopy(ips_to_reduce) int_ip = int(binary_ip, 2)
reduction_limit_reached = False ip_str = str(ipaddress.IPv4Address(int_ip))
while len(ips) > max_len and not reduction_limit_reached: return f"{ip_str}/{vlsm}"
reduced_ips = map(reduce_prefixlen, ips)
merged_ips = cidr_merge(reduced_ips)
if len(merged_ips) > 1:
ips = merged_ips
else:
reduction_limit_reached = True
print("The reduction limit has been reached")
return ips
def main(): def main():
file = "cidr4.txt" file = "cidr4.txt"
ips = get_ips(file) data = get_data(file)
print(f"{len(ips)=}")
merged_ips = merge_ips(ips, 10)
print(f"{len(merged_ips)=}")
print(", ".join([str(x) for x in merged_ips]))
if __name__ == "__main__": if __name__ == "__main__":
assert reduce_prefixlen(IPNetwork("4.78.139.0/24")) == IPNetwork("4.78.138.0/23") assert cidr4_to_binary("4.78.139.0/24") == ("100010011101000101100000000", 24)
assert binary_to_cidr4("100010011101000101100000000", 24) == "4.78.139.0/24"
main() main()