Skip to content

Commit 8b4ade3

Browse files
thomas-manginclaude
andcommitted
Refactor Wave 5 NLRI qualifiers to packed-bytes-first pattern
Refactor all 5 NLRI qualifier classes to store packed bytes as primary representation with factory methods for semantic construction: - PathInfo: make_from_integer(), make_from_ip() - RouteDistinguisher: make_from_elements() (removed fromElements alias) - Labels: make_labels(list[int], bos) - ESI: make_default(), make_esi() - EthernetTag: make_etag(int) Each class now: - Takes packed bytes in __init__ with size validation - Provides factory methods for semantic construction - Uses @Property for backward-compatible attribute access - Unpacks data on demand rather than storing both forms 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
1 parent 1bf1f40 commit 8b4ade3

File tree

18 files changed

+592
-520
lines changed

18 files changed

+592
-520
lines changed

.claude/docs/plans/packed-attribute.md

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -108,8 +108,15 @@ class INET(NLRI):
108108
### Wave 4: MP Attributes + BGP-LS + SR
109109
21-55. `mprnlri.py`, `mpurnlri.py`, `bgpls/*.py`, `sr/*.py`
110110

111-
### Wave 5: Qualifiers
112-
56-60. `nlri/qualifier/path.py`, `rd.py`, `labels.py`, `esi.py`, `etag.py`
111+
### Wave 5: Qualifiers ✅ COMPLETE
112+
113+
| File | Status | Factory Method |
114+
|------|--------|----------------|
115+
| `path.py` | ✅ Done | `PathInfo.make_from_integer(int)`, `PathInfo.make_from_ip(str)` |
116+
| `rd.py` | ✅ Done | `RouteDistinguisher.make_from_elements(prefix, suffix)` |
117+
| `labels.py` | ✅ Done | `Labels.make_labels(list[int], bos)` |
118+
| `esi.py` | ✅ Done | `ESI.make_default()`, `ESI.make_esi(bytes)` |
119+
| `etag.py` | ✅ Done | `EthernetTag.make_etag(int)` |
113120

114121
### Wave 6: NLRI Types
115122
61. **`src/exabgp/bgp/message/update/nlri/cidr.py`**

src/exabgp/bgp/message/update/nlri/inet.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,7 @@ def unpack_nlri(
164164
break
165165
if label & LABEL_BOTTOM_OF_STACK_BIT:
166166
break
167-
nlri.labels = Labels(labels)
167+
nlri.labels = Labels.make_labels(labels)
168168

169169
if rd_size:
170170
mask -= rd_mask # the route distinguisher

src/exabgp/bgp/message/update/nlri/qualifier/esi.py

Lines changed: 24 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77
"""
88

99
from __future__ import annotations
10-
from typing import Type
1110

1211
# TODO: take into account E-VPN specs that specify the role of the first bit of ESI
1312
# (since draft-ietf-l2vpn-evpn-05)
@@ -20,15 +19,29 @@ class ESI:
2019
DEFAULT = bytes([0x00] * LENGTH) # All zeros
2120
MAX = bytes([0xFF] * LENGTH) # All ones
2221

23-
def __init__(self, esi: bytes | None = None) -> None:
24-
self.esi: bytes = self.DEFAULT if esi is None else esi
25-
if len(self.esi) != self.LENGTH:
26-
raise Exception(f'incorrect ESI, len {len(esi)} instead of {self.LENGTH}') # type: ignore[arg-type]
22+
def __init__(self, packed: bytes) -> None:
23+
if len(packed) != self.LENGTH:
24+
raise ValueError(f'ESI requires exactly {self.LENGTH} bytes, got {len(packed)}')
25+
self._packed = packed
26+
27+
@classmethod
28+
def make_esi(cls, esi_bytes: bytes) -> 'ESI':
29+
"""Create ESI from bytes."""
30+
return cls(esi_bytes)
31+
32+
@classmethod
33+
def make_default(cls) -> 'ESI':
34+
"""Create ESI with default value (all zeros)."""
35+
return cls(cls.DEFAULT)
36+
37+
@property
38+
def esi(self) -> bytes:
39+
return self._packed
2740

2841
def __eq__(self, other: object) -> bool:
2942
if not isinstance(other, ESI):
3043
return False
31-
return self.esi == other.esi
44+
return self._packed == other._packed
3245

3346
def __lt__(self, other: object) -> bool:
3447
raise RuntimeError('comparing ESI for ordering does not make sense')
@@ -43,24 +56,24 @@ def __ge__(self, other: object) -> bool:
4356
raise RuntimeError('comparing ESI for ordering does not make sense')
4457

4558
def __str__(self) -> str:
46-
if self.esi == self.DEFAULT:
59+
if self._packed == self.DEFAULT:
4760
return '-'
48-
return ':'.join('{:02x}'.format(_) for _ in self.esi)
61+
return ':'.join('{:02x}'.format(_) for _ in self._packed)
4962

5063
def __repr__(self) -> str:
5164
return self.__str__()
5265

5366
def pack_esi(self) -> bytes:
54-
return self.esi
67+
return self._packed
5568

5669
def __len__(self) -> int:
5770
return self.LENGTH
5871

5972
def __hash__(self) -> int:
60-
return hash(self.esi)
73+
return hash(self._packed)
6174

6275
@classmethod
63-
def unpack_esi(cls: Type[ESI], data: bytes) -> ESI:
76+
def unpack_esi(cls, data: bytes) -> 'ESI':
6477
return cls(data[: cls.LENGTH])
6578

6679
def json(self, compact: bool = False) -> str:

src/exabgp/bgp/message/update/nlri/qualifier/etag.py

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -10,22 +10,33 @@
1010
# (since draft-ietf-l2vpn-evpn-05)
1111

1212
from __future__ import annotations
13-
from typing import Type
1413

1514
from struct import pack
1615
from struct import unpack
1716

1817

1918
class EthernetTag:
2019
MAX = pow(2, 32) - 1
20+
LENGTH = 4
2121

22-
def __init__(self, tag: int = 0) -> None:
23-
self.tag: int = tag
22+
def __init__(self, packed: bytes) -> None:
23+
if len(packed) != self.LENGTH:
24+
raise ValueError(f'EthernetTag requires exactly {self.LENGTH} bytes, got {len(packed)}')
25+
self._packed = packed
26+
27+
@classmethod
28+
def make_etag(cls, tag: int) -> 'EthernetTag':
29+
"""Create EthernetTag from integer value."""
30+
return cls(pack('!L', tag))
31+
32+
@property
33+
def tag(self) -> int:
34+
return unpack('!L', self._packed)[0]
2435

2536
def __eq__(self, other: object) -> bool:
2637
if not isinstance(other, EthernetTag):
2738
return False
28-
return self.tag == other.tag
39+
return self._packed == other._packed
2940

3041
def __lt__(self, other: object) -> bool:
3142
raise RuntimeError('comparing EthernetTag for ordering does not make sense')
@@ -46,17 +57,17 @@ def __repr__(self) -> str:
4657
return repr(self.tag)
4758

4859
def pack_etag(self) -> bytes:
49-
return pack('!L', self.tag)
60+
return self._packed
5061

5162
def __len__(self) -> int:
52-
return 4
63+
return self.LENGTH
5364

5465
def __hash__(self) -> int:
55-
return hash(self.tag)
66+
return hash(self._packed)
5667

5768
@classmethod
58-
def unpack_etag(cls: Type[EthernetTag], data: bytes) -> EthernetTag:
59-
return cls(unpack('!L', data[:4])[0])
69+
def unpack_etag(cls, data: bytes) -> 'EthernetTag':
70+
return cls(data[: cls.LENGTH])
6071

6172
def json(self, compact: bool = False) -> str:
6273
return '"ethernet-tag": {}'.format(self.tag)

src/exabgp/bgp/message/update/nlri/qualifier/labels.py

Lines changed: 75 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
"""
77

88
from __future__ import annotations
9-
from typing import ClassVar, Type
9+
from typing import ClassVar
1010

1111
from struct import pack
1212
from struct import unpack
@@ -25,98 +25,120 @@ class Labels:
2525

2626
NOLABEL: ClassVar['Labels']
2727

28-
def __init__(self, labels: list[int], bos: bool = True, raw_labels: list[int] | None = None) -> None:
29-
self.labels: list[int] = labels
30-
self.raw_labels: list[int] = raw_labels if raw_labels else []
31-
packed = []
32-
if raw_labels:
33-
for label in raw_labels:
34-
packed.append(pack('!L', label)[1:])
35-
# fill self.labels as well, not for packing, but to allow
36-
# consistent string representations
37-
if not self.labels:
38-
self.labels = [x >> 4 for x in raw_labels]
39-
else:
40-
for label in labels:
41-
# shift to 20 bits of the label to be at the top of three bytes and then truncate.
42-
packed.append(pack('!L', label << 4)[1:])
43-
# Mark the bottom of stack with the bit
44-
if packed and bos:
45-
packed.pop()
46-
packed.append(pack('!L', (label << 4) | 1)[1:])
47-
self.raw_labels = [0 for _ in self.labels]
48-
self.packed: bytes = b''.join(packed)
49-
self._len: int = len(self.packed)
28+
def __init__(self, packed: bytes) -> None:
29+
if len(packed) % 3 != 0:
30+
raise ValueError(f'Labels packed data must be multiple of 3 bytes, got {len(packed)}')
31+
self._packed = packed
32+
33+
@classmethod
34+
def make_labels(cls, labels: list[int], bos: bool = True) -> 'Labels':
35+
"""Create Labels from list of label integers."""
36+
if not labels:
37+
return cls(b'')
38+
packed_parts = []
39+
for i, label in enumerate(labels):
40+
# shift to 20 bits of the label to be at the top of three bytes
41+
value = label << 4
42+
# Mark the bottom of stack with the bit on last label
43+
if bos and i == len(labels) - 1:
44+
value |= 1
45+
packed_parts.append(pack('!L', value)[1:])
46+
return cls(b''.join(packed_parts))
47+
48+
@property
49+
def labels(self) -> list[int]:
50+
"""Extract label values from packed bytes."""
51+
result = []
52+
data = self._packed
53+
while len(data) >= 3:
54+
raw = unpack('!L', bytes([0]) + data[:3])[0]
55+
result.append(raw >> 4)
56+
data = data[3:]
57+
return result
58+
59+
@property
60+
def raw_labels(self) -> list[int]:
61+
"""Extract raw 24-bit values (includes TC/S bits) from packed bytes."""
62+
result = []
63+
data = self._packed
64+
while len(data) >= 3:
65+
raw = unpack('!L', bytes([0]) + data[:3])[0]
66+
result.append(raw)
67+
data = data[3:]
68+
return result
5069

5170
def __eq__(self, other: object) -> bool:
5271
if not isinstance(other, Labels):
5372
return False
54-
return self.labels == other.labels
73+
return self._packed == other._packed
5574

5675
def __lt__(self, other: object) -> bool:
57-
raise RuntimeError('comparing EthernetTag for ordering does not make sense')
76+
raise RuntimeError('comparing Labels for ordering does not make sense')
5877

5978
def __le__(self, other: object) -> bool:
60-
raise RuntimeError('comparing EthernetTag for ordering does not make sense')
79+
raise RuntimeError('comparing Labels for ordering does not make sense')
6180

6281
def __gt__(self, other: object) -> bool:
63-
raise RuntimeError('comparing EthernetTag for ordering does not make sense')
82+
raise RuntimeError('comparing Labels for ordering does not make sense')
6483

6584
def __ge__(self, other: object) -> bool:
66-
raise RuntimeError('comparing EthernetTag for ordering does not make sense')
85+
raise RuntimeError('comparing Labels for ordering does not make sense')
6786

6887
def pack_labels(self) -> bytes:
69-
return self.packed
88+
return self._packed
7089

7190
def __len__(self) -> int:
72-
return self._len
91+
return len(self._packed)
7392

7493
def json(self) -> str:
75-
if len(self.labels) >= 1:
94+
labels = self.labels
95+
raw_labels = self.raw_labels
96+
if len(labels) >= 1:
7697
return '"label": [ {} ]'.format(
7798
', '.join(
78-
[
79-
'[%d%s]' % (label, opt_raw_label(raw, ', %d'))
80-
for (label, raw) in zip(self.labels, self.raw_labels)
81-
],
99+
['[%d%s]' % (label, opt_raw_label(raw, ', %d')) for (label, raw) in zip(labels, raw_labels)],
82100
)
83101
)
84102
return ''
85103

86104
def __str__(self) -> str:
87-
if len(self.labels) > 1:
105+
labels = self.labels
106+
raw_labels = self.raw_labels
107+
if len(labels) > 1:
88108
return ' label [ {} ]'.format(
89109
' '.join(
90-
['%d%s' % (label, opt_raw_label(raw)) for (label, raw) in zip(self.labels, self.raw_labels)],
110+
['%d%s' % (label, opt_raw_label(raw)) for (label, raw) in zip(labels, raw_labels)],
91111
)
92112
)
93-
if len(self.labels) == 1:
94-
return ' label %d%s' % (self.labels[0], opt_raw_label(self.raw_labels[0]))
113+
if len(labels) == 1:
114+
return ' label %d%s' % (labels[0], opt_raw_label(raw_labels[0]))
95115
return ''
96116

97117
def __repr__(self) -> str:
98-
if len(self.labels) > 1:
118+
labels = self.labels
119+
raw_labels = self.raw_labels
120+
if len(labels) > 1:
99121
return '[ {} ]'.format(
100122
','.join(
101-
['%d%s' % (label, opt_raw_label(raw)) for (label, raw) in zip(self.labels, self.raw_labels)],
123+
['%d%s' % (label, opt_raw_label(raw)) for (label, raw) in zip(labels, raw_labels)],
102124
)
103125
)
104-
if len(self.labels) == 1:
105-
return '%d%s' % (self.labels[0], opt_raw_label(self.raw_labels[0]))
126+
if len(labels) == 1:
127+
return '%d%s' % (labels[0], opt_raw_label(raw_labels[0]))
106128
return '[ ]'
107129

108130
@classmethod
109-
def unpack_labels(cls: Type[Labels], data: bytes) -> Labels:
110-
labels = []
111-
raw_labels = []
112-
while len(data):
113-
label = unpack('!L', bytes([0]) + data[:3])[0]
131+
def unpack_labels(cls, data: bytes) -> 'Labels':
132+
"""Unpack labels from data, stopping at bottom-of-stack bit."""
133+
packed_parts = []
134+
while len(data) >= 3:
135+
chunk = data[:3]
136+
packed_parts.append(chunk)
114137
data = data[3:]
115-
labels.append(label >> 4)
116-
raw_labels.append(label)
117-
if label & 0x001:
138+
raw = unpack('!L', bytes([0]) + chunk)[0]
139+
if raw & 0x001: # bottom-of-stack bit
118140
break
119-
return cls(labels, raw_labels=raw_labels)
141+
return cls(b''.join(packed_parts))
120142

121143

122-
Labels.NOLABEL = Labels([])
144+
Labels.NOLABEL = Labels(b'')

0 commit comments

Comments
 (0)