-
Notifications
You must be signed in to change notification settings - Fork 38
Add MSTP implementation and update network/frontend files #380
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
back/src/net_utils/mstp.py
Outdated
| """ | ||
| MSTP (Multiple Spanning Tree Protocol) configuration utilities. | ||
| MSTP allows mapping multiple VLANs to different spanning tree instances, | ||
| providing load balancing and redundancy for VLAN traffic. | ||
| For MSTP we use Linux bridge with mstpd instead of OVS, | ||
| because OVS doesn't support true MSTP with multiple spanning tree instances. | ||
| """ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Мне кажется это можно убрать
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes
back/src/net_utils/mstp.py
Outdated
|
|
||
|
|
||
| def setup_mstp(net: IPNet, nodes: list[Node]) -> None: | ||
| """Configure MSTP on switches that have stp=3. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Тоже можно сократить, слишком комментариев много, они не помогают лучше разобраться
back/src/net_utils/mstp.py
Outdated
| configure_mstp_bridge(switch, node) | ||
|
|
||
|
|
||
| def configure_mstp_bridge(switch, node: Node) -> None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Аннотацию для параметра switch надо добавить. Вообще прогони PR через mypy
back/src/net_utils/mstp.py
Outdated
| # Ensure priority is in valid range 0-15 | ||
| prio = min(15, max(0, priority)) | ||
| switch.cmd(f"mstpctl settreeprio {bridge_name} {instance_id} " f"{prio}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
А точно нужно priority подгонять под правильное значение? Может лучше если оно выходит за рамки бросать ошибку
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
“I intentionally kept the MST priority clamping in the code. If a user sets a value outside the valid range (0–15), the code automatically clamps it to the nearest valid value.
This ensures the network topology always comes up and avoids runtime errors from misconfigured priorities.
I added a warning to inform users when clamping occurs, so it’s visible and debuggable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This ensures the network topology always comes up and avoids runtime errors from misconfigured priorities
А эти свойства пользователь задаёт или кто? Просто если пользователь, то он наверное ожидает, что будет такое свойство, как он написал, либо ошибка.
И ещё... чья это цитата?
back/src/network_schema.py
Outdated
| @dataclass | ||
| class MstInstance: | ||
| """ | ||
| Represents an MST (Multiple Spanning Tree) instance configuration. | ||
| Attributes: | ||
| instance_id (int): MST instance ID (0-64). | ||
| vlans (list[int]): List of VLAN IDs mapped to this instance. | ||
| priority (Optional[int]): Bridge priority for this MST instance. | ||
| """ | ||
|
|
||
| instance_id: int | ||
| vlans: list[int] | ||
| priority: Optional[int] = None | ||
|
|
||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ты уверен что этот класс необходим? И если необходим, то он точно нужен именно в этом файле?
Просто в этом файле определены глобальные абстракции типо вершины, ребра, задачи. В нём нет ничего связанного с конкретными протоколами
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
А за что эти три файла удалены были?
| result = switch.cmd("which mstpctl 2>/dev/null || echo 'not found'") | ||
|
|
||
| if "not found" in result: | ||
| print(f"Warning: mstpctl not found, using STP fallback for " f"{switch.name}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
print не нужен наверное тут
MSTP is working on VLANs
Why this proves MSTP works on VLANs
sudo mstpctl showvid2fid br-test→ VLAN → FID mapping.sudo mstpctl showfid2mstid br-test→ FID → MSTI mapping.“VLANs 10 and 20 are mapped to FID 1, which is managed by MSTI 1. MSTP will manage these VLANs independently of the default spanning tree.”