Skip to content

Commit 849a96f

Browse files
thomas-manginclaude
thomas-mangin
and
claude
committed
refactor: Complete VPLS immutability - remove builder mode (Phase 5)
Migrate announce parsers to Settings mode: - announce/path.py uses nlri_class=INET, settings_class=INETSettings - announce/label.py uses nlri_class=Label, settings_class=INETSettings - announce/vpn.py uses nlri_class=IPVPN, settings_class=INETSettings Remove VPLS builder mode entirely: - Remove make_empty() factory method - Remove assign() method override - Remove builder mode slots (_rd, _endpoint, _base, _offset, _size_value) - Simplify properties to always unpack from _packed bytes - Remove property setters (rd, endpoint, base, offset, size) - Simplify _pack_nlri_simple() to just return self._packed - Simplify feedback() - only check nexthop and size consistency - Update __copy__ and __deepcopy__ for new slot structure Remove deprecated tests: - Remove TestVPLSAssign class (tested make_empty + assign) - Remove feedback tests for missing fields (builder mode only) VPLS is now fully immutable - all instances created from packed bytes. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent 0cb7cca commit 849a96f

File tree

7 files changed

+258
-268
lines changed

7 files changed

+258
-268
lines changed

‎plan/nlri-immutability-refactoring.md‎

Lines changed: 37 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -853,12 +853,44 @@ Full removal requires:
853853
1. ~~Block syntax `vpls site5 { ... }` via `ParseVPLS.pre()`~~ ✅ Fixed
854854
2. ~~Section parser `nlri-set` actions → `scope.nlri_assign()`~~ ✅ Fixed (settings mode)
855855
3. Legacy mode in RouteBuilderValidator (`_validate_legacy`) - for non-VPLS types
856-
4. Unit tests in `tests/unit/test_vpls.py` that test the legacy pattern
856+
4. ~~Unit tests in `tests/unit/test_vpls.py` that test the legacy pattern~~ ✅ Removed
857857

858-
**Resume Point:**
859-
- Update `announce/path.py`, `announce/vpn.py`, `announce/label.py` to use Settings
860-
- Update unit tests to use Settings pattern only
861-
- Remove deprecated `make_empty()` and `assign()` methods from VPLS
858+
### 2025-12-07 - Phase 5 Continued: Full VPLS Immutability
859+
860+
**Completed:**
861+
- ✅ Updated `announce/path.py` to use Settings mode (`nlri_class=INET, settings_class=INETSettings`)
862+
- ✅ Updated `announce/label.py` to use Settings mode (`nlri_class=Label, settings_class=INETSettings`)
863+
- ✅ Updated `announce/vpn.py` to use Settings mode (`nlri_class=IPVPN, settings_class=INETSettings`)
864+
- ✅ Removed builder mode tests from `test_vpls.py` (TestVPLSAssign class, feedback tests for missing fields)
865+
- ✅ Removed `make_empty()` method from VPLS
866+
- ✅ Removed `assign()` override from VPLS
867+
- ✅ Removed builder mode slots from VPLS (`_rd`, `_endpoint`, `_base`, `_offset`, `_size_value`)
868+
- ✅ Simplified VPLS properties to always use packed bytes (no fallback to builder storage)
869+
- ✅ Removed property setters from VPLS (rd, endpoint, base, offset, size)
870+
- ✅ Simplified `_pack_nlri_simple()` to just return `self._packed`
871+
- ✅ Simplified `feedback()` to only check nexthop and size consistency
872+
- ✅ Updated `__copy__` and `__deepcopy__` to not copy removed slots
873+
- ✅ All tests pass (11 suites, 42.9s)
874+
875+
**Files modified:**
876+
- `src/exabgp/configuration/announce/path.py` - Use Settings mode
877+
- `src/exabgp/configuration/announce/label.py` - Use Settings mode
878+
- `src/exabgp/configuration/announce/vpn.py` - Use Settings mode
879+
- `src/exabgp/bgp/message/update/nlri/vpls.py` - Full immutability (no builder mode)
880+
- `tests/unit/test_vpls.py` - Removed builder mode tests
881+
882+
**Phase 5 COMPLETE for VPLS!**
883+
884+
**VPLS is now fully immutable:**
885+
- All instances created with packed bytes
886+
- No mutation after construction
887+
- Properties unpack lazily from wire bytes
888+
- `make_vpls()` and `from_settings()` are the only factory methods
889+
890+
**What Still Uses Mutation (non-VPLS):**
891+
- INET/Label/IPVPN still support mutation via base class `assign()` for ParseStaticRoute
892+
- ParseStaticRoute Section parser still uses legacy mode
893+
- FlowSpec builder pattern (rules added incrementally)
862894

863895
---
864896

‎plan/packed-nlri-conversion.md‎

Lines changed: 160 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,160 @@
1+
# Plan: Convert NLRI Classes to Packed-Bytes-First Pattern
2+
3+
**Status:** 🔄 Ready to start
4+
**Priority:** 🟡 Medium
5+
**Command:** `/convert-nlri <nlri-name>`
6+
**Reference:** `.claude/exabgp/PACKED_BYTES_FIRST_PATTERN.md`
7+
**Prerequisite:** `plan/nlri-immutability-refactoring.md` (Phase 5 complete for VPLS)
8+
9+
---
10+
11+
## Overview
12+
13+
Convert all NLRI classes to the packed-bytes-first pattern to:
14+
- Reduce memory allocation (store wire bytes directly)
15+
- Enable lazy unpacking (parse fields only when accessed)
16+
- Support zero-copy routing (return stored bytes without re-packing)
17+
18+
## Key Principles
19+
20+
1. **`_packed` is ALWAYS required** - no `None`, no conditionals in properties
21+
2. **Two entry points only:**
22+
- `unpack_nlri()` - from existing wire Buffer (don't modify, just store)
23+
- `make_xxx()` - build new packed bytes from components
24+
3. **`@property` for all fields** - simple return, no conditionals
25+
4. **No `make_empty()`** - breaks immutability
26+
27+
## Prerequisite: NLRI Immutability Refactoring
28+
29+
**See:** `plan/nlri-immutability-refactoring.md`
30+
31+
Configuration parser refactoring status:
32+
- Phase 1: ✅ COMPLETE - Add factory methods to NLRI classes
33+
- Phase 2: ✅ COMPLETE - RouteBuilderValidator deferred construction
34+
- Phase 3: ✅ COMPLETE - Static route deferred construction
35+
- Phase 4: ✅ COMPLETE - TypeSelectorValidator deferred construction
36+
- Phase 5: ✅ COMPLETE for VPLS - Remove mutation support
37+
38+
**VPLS is fully immutable and can be used as reference implementation.**
39+
40+
---
41+
42+
## Conversion Status
43+
44+
### Core NLRI Classes
45+
46+
| Class | File | Has Tests | Converted | Notes |
47+
|-------|------|-----------|-----------|-------|
48+
| VPLS | `nlri/vpls.py` | ✅ | ✅ | Reference implementation |
49+
| RTC | `nlri/rtc.py` | ✅ | ❌ | Partial - rt needs negotiated |
50+
| Flow | `nlri/flow.py` | ❌ | ❌ | Builder pattern - not applicable |
51+
| INET | `nlri/inet.py` | ❌ | ❌ | Base for Label/IPVPN |
52+
| Label | `nlri/label.py` | ❌ | ❌ | Extends INET |
53+
| IPVPN | `nlri/ipvpn.py` | ❌ | ❌ | Extends Label |
54+
55+
### EVPN Classes
56+
57+
| Class | File | Has Tests | Converted | Notes |
58+
|-------|------|-----------|-----------|-------|
59+
| EVPN (base) | `evpn/nlri.py` | ❌ | ❌ | Base class |
60+
| GenericEVPN | `evpn/nlri.py` | ❌ | ❌ | Fallback handler |
61+
| MAC | `evpn/mac.py` | ❌ | ❌ | Type 2 |
62+
| EthernetAD | `evpn/ethernetad.py` | ❌ | ❌ | Type 1 |
63+
| Multicast | `evpn/multicast.py` | ❌ | ❌ | Type 3 |
64+
| EthernetSegment | `evpn/segment.py` | ❌ | ❌ | Type 4 |
65+
| Prefix | `evpn/prefix.py` | ❌ | ❌ | Type 5 |
66+
67+
### MVPN Classes
68+
69+
| Class | File | Has Tests | Converted | Notes |
70+
|-------|------|-----------|-----------|-------|
71+
| MVPN (base) | `mvpn/nlri.py` | ❌ | ❌ | Base class |
72+
| GenericMVPN | `mvpn/nlri.py` | ❌ | ❌ | Fallback handler |
73+
| SourceAD | `mvpn/sourcead.py` | ❌ | ❌ | Type 1 |
74+
| SharedJoin | `mvpn/sharedjoin.py` | ❌ | ❌ | Type 6 |
75+
| SourceJoin | `mvpn/sourcejoin.py` | ❌ | ❌ | Type 7 |
76+
77+
### MUP Classes
78+
79+
| Class | File | Has Tests | Converted | Notes |
80+
|-------|------|-----------|-----------|-------|
81+
| MUP (base) | `mup/nlri.py` | ❌ | ❌ | Base class |
82+
| GenericMUP | `mup/nlri.py` | ❌ | ❌ | Fallback handler |
83+
| InterworkSegmentDiscoveryRoute | `mup/isd.py` | ❌ | ❌ | ISD |
84+
| DirectSegmentDiscoveryRoute | `mup/dsd.py` | ❌ | ❌ | DSD |
85+
| Type1SessionTransformedRoute | `mup/t1st.py` | ❌ | ❌ | T1ST |
86+
| Type2SessionTransformedRoute | `mup/t2st.py` | ❌ | ❌ | T2ST |
87+
88+
### BGP-LS Classes
89+
90+
| Class | File | Has Tests | Converted | Notes |
91+
|-------|------|-----------|-----------|-------|
92+
| BGPLS (base) | `bgpls/nlri.py` | ❌ | ❌ | Base class |
93+
| GenericBGPLS | `bgpls/nlri.py` | ❌ | ❌ | Fallback handler |
94+
| NODE | `bgpls/node.py` | ❌ | ❌ | Node descriptor |
95+
| LINK | `bgpls/link.py` | ❌ | ❌ | Link descriptor |
96+
| PREFIXv4 | `bgpls/prefixv4.py` | ❌ | ❌ | IPv4 prefix |
97+
| PREFIXv6 | `bgpls/prefixv6.py` | ❌ | ❌ | IPv6 prefix |
98+
| SRv6SID | `bgpls/srv6sid.py` | ❌ | ❌ | SRv6 SID |
99+
100+
---
101+
102+
## Conversion Order (Recommended)
103+
104+
1. **Simple fixed-length first:**
105+
- RTC (already partial)
106+
- MVPN types (fixed format)
107+
- MUP types
108+
109+
2. **EVPN types:**
110+
- Variable length but well-defined structure
111+
112+
3. **BGP-LS types:**
113+
- Complex TLV structure
114+
115+
4. **Skip or document as not applicable:**
116+
- Flow (builder pattern with rules)
117+
- INET/Label/IPVPN (inheritance hierarchy - needs careful analysis)
118+
119+
---
120+
121+
## How to Convert
122+
123+
Use the slash command:
124+
```
125+
/convert-nlri <nlri-name>
126+
```
127+
128+
Examples:
129+
- `/convert-nlri rtc`
130+
- `/convert-nlri evpn/mac`
131+
- `/convert-nlri mvpn/sourcead`
132+
- `/convert-nlri mup/isd`
133+
134+
The command will:
135+
1. Read and analyze the NLRI structure
136+
2. Check for / create unit tests
137+
3. Convert to packed-bytes-first pattern
138+
4. Run tests to verify
139+
140+
---
141+
142+
## Progress Log
143+
144+
### 2025-12-07
145+
- Created conversion plan
146+
- Created `/convert-nlri` slash command
147+
- VPLS already converted (reference implementation)
148+
149+
### 2025-12-07 - Phase 5 Verified
150+
- Verified Phase 4 of prerequisite plan (TypeSelectorValidator)
151+
- Completed Phase 5: VPLS immutability
152+
- Updated announce/path.py, label.py, vpn.py to use Settings mode
153+
- Removed deprecated make_empty() and assign() from VPLS
154+
- Removed builder mode slots and simplified properties
155+
- All 11 tests pass
156+
- VPLS is now fully immutable and ready as reference implementation
157+
158+
---
159+
160+
**Updated:** 2025-12-07

0 commit comments

Comments
 (0)