Skip to content

Commit 3a53b2a

Browse files
thomas-manginclaude
andcommitted
refactor: Apply packed-bytes-first pattern to Label class (Phase 2)
Label NLRI now stores complete wire format in _packed: - [addpath:4?][mask:1][labels:3n][prefix:var] - Removed _labels_packed slot, added _has_labels flag - Added _label_end_offset property for BOS scanning - labels and cidr properties extract from _packed lazily - pack_nlri() returns _packed directly (zero-copy) - Updated from_cidr() to build wire format with labels - Updated __hash__, __eq__, __len__ for new storage - Updated __copy__ and __deepcopy__ to copy _has_labels IPVPN updated to inherit Label changes: - Uses _has_labels instead of _labels_packed - pack_nlri() recalculates mask to include RD bits - unpack_nlri() stores labels in _packed format 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent e1f7e39 commit 3a53b2a

File tree

3 files changed

+240
-105
lines changed

3 files changed

+240
-105
lines changed

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

Lines changed: 92 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -68,19 +68,20 @@
6868
| Assigned (2 octets) | <- Admin-assigned value
6969
+---------------------------+
7070
71-
Wire Format (_packed):
72-
=====================
73-
This class stores ONLY the CIDR payload in _packed (not the full VPN NLRI).
71+
Wire Format (_packed) - Packed-Bytes-First Pattern (Partial):
72+
=============================================================
73+
This class inherits Label's packed-bytes-first pattern but keeps RD separate.
7474
75-
_packed stores: [mask_byte][truncated_ip_bytes...] (same as INET/Label)
76-
self.labels stores: Labels object with the MPLS label stack
77-
self.rd stores: RouteDistinguisher object
75+
_packed stores: [addpath:4?][mask:1][labels:3n][prefix:var] (inherited from Label)
76+
- mask = combined mask for labels + prefix (NOT including RD bits)
77+
- labels = raw MPLS label bytes
78+
- prefix = truncated IP prefix bytes
7879
79-
On pack_nlri(), these are combined:
80-
output = [length][labels][rd][prefix]
81-
where length = labels*24 + 64 + mask
80+
_rd_packed stores: Route Distinguisher bytes (8 bytes)
8281
83-
Note: path_info (ADD-PATH) is stored in self.path_info, NOT in _packed.
82+
pack_nlri() must recalculate mask to include RD bits and insert RD.
83+
84+
Note: Phase 3 will move RD into _packed for full zero-copy.
8485
8586
Class Hierarchy:
8687
===============
@@ -126,11 +127,14 @@
126127
@NLRI.register(AFI.ipv4, SAFI.mpls_vpn)
127128
@NLRI.register(AFI.ipv6, SAFI.mpls_vpn)
128129
class IPVPN(Label):
129-
"""IPVPN NLRI with separate storage for CIDR, labels, and RD.
130+
"""IPVPN NLRI inheriting Label's packed-bytes-first pattern with separate RD.
131+
132+
Wire format for pack: [addpath?][mask][labels][rd][prefix]
133+
Storage:
134+
- _packed: [addpath:4?][mask:1][labels:3n][prefix:var] (inherited from Label)
135+
- _rd_packed: Route Distinguisher bytes (8 bytes)
130136
131-
Wire format: [mask][labels][rd][prefix]
132-
Storage: _packed (CIDR), _labels_packed (labels bytes), _rd_packed (RD bytes)
133-
pack_nlri() = concatenation of all parts with computed mask
137+
pack_nlri() must recalculate mask to include RD bits and insert RD.
134138
135139
Uses class-level SAFI (always mpls_vpn) - no instance storage needed.
136140
"""
@@ -145,7 +149,7 @@ def __init__(self, packed: bytes, afi: AFI, *, has_addpath: bool = False) -> Non
145149
"""Create an IPVPN NLRI from packed wire format bytes.
146150
147151
Args:
148-
packed: Wire format bytes [addpath:4?][mask:1][prefix:var]
152+
packed: Wire format bytes [addpath:4?][mask:1][labels:3n][prefix:var]
149153
afi: Address Family Identifier
150154
has_addpath: If True, packed includes 4-byte path identifier at start
151155
@@ -186,22 +190,29 @@ def from_cidr(
186190
Returns:
187191
New IPVPN instance with SAFI=mpls_vpn
188192
"""
189-
# Build wire format: [addpath:4?][mask:1][prefix:var]
190-
# Note: Labels and RD are stored separately in _labels_packed and _rd_packed for now
191-
cidr_packed = cidr.pack_nlri()
193+
# Build wire format: [addpath:4?][mask:1][labels:3n][prefix:var]
194+
# Note: RD is stored separately in _rd_packed (Phase 3 will move to _packed)
195+
labels_packed = labels.pack_labels() if labels is not None else b''
196+
has_labels = len(labels_packed) > 0
197+
combined_mask = len(labels_packed) * 8 + cidr.mask
198+
prefix_bytes = cidr.pack_ip()
199+
200+
# Build packed data: [mask][labels][prefix]
201+
nlri_bytes = bytes([combined_mask]) + labels_packed + prefix_bytes
202+
192203
has_addpath = path_info is not PathInfo.DISABLED
193204
if has_addpath:
194-
packed = bytes(path_info.pack_path()) + cidr_packed
205+
packed = bytes(path_info.pack_path()) + nlri_bytes
195206
else:
196-
packed = cidr_packed
207+
packed = nlri_bytes
197208

198209
instance = object.__new__(cls)
199210
# Note: safi parameter is ignored - IPVPN.safi is a class-level property
200211
NLRI.__init__(instance, afi, cls.safi, action)
201212
instance._packed = packed
202213
instance._has_addpath = has_addpath
214+
instance._has_labels = has_labels
203215
instance.nexthop = IP.NoNextHop
204-
instance._labels_packed = labels.pack_labels() if labels is not None else b''
205216
instance._rd_packed = rd.pack_rd() if rd is not None else b''
206217
return instance
207218

@@ -278,8 +289,8 @@ def __repr__(self) -> str:
278289
return self.extensive()
279290

280291
def __len__(self) -> int:
281-
# Total length = _packed (includes addpath if present) + labels + rd
282-
return len(self._packed) + len(self._labels_packed) + len(self._rd_packed)
292+
# Total length = _packed (includes addpath, mask, labels, prefix) + rd
293+
return len(self._packed) + len(self._rd_packed)
283294

284295
def __eq__(self, other: Any) -> bool:
285296
return Label.__eq__(self, other) and self._rd_packed == other._rd_packed
@@ -288,14 +299,10 @@ def __ne__(self, other: Any) -> bool:
288299
return not self.__eq__(other)
289300

290301
def __hash__(self) -> int:
291-
if self.path_info is PathInfo.NOPATH:
292-
addpath = b'no-pi'
293-
elif self.path_info is PathInfo.DISABLED:
294-
addpath = b'disabled'
295-
else:
296-
addpath = self.path_info.pack_path()
297-
mask = len(self._labels_packed) * 8 + len(self._rd_packed) * 8 + self.cidr.mask
298-
return hash(addpath + bytes([mask]) + self._labels_packed + self._rd_packed + self.cidr.pack_ip())
302+
# Include RD in hash along with _packed (which has labels)
303+
if self._has_addpath:
304+
return hash(self._packed + self._rd_packed)
305+
return hash(b'disabled' + self._packed + self._rd_packed)
299306

300307
def __copy__(self) -> 'IPVPN':
301308
new = self.__class__.__new__(self.__class__)
@@ -306,7 +313,7 @@ def __copy__(self) -> 'IPVPN':
306313
# INET slots
307314
new._has_addpath = self._has_addpath
308315
# Label slots
309-
new._labels_packed = self._labels_packed
316+
new._has_labels = self._has_labels
310317
# IPVPN slots
311318
new._rd_packed = self._rd_packed
312319
return new
@@ -321,7 +328,7 @@ def __deepcopy__(self, memo: dict[Any, Any]) -> 'IPVPN':
321328
# INET slots
322329
new._has_addpath = self._has_addpath # bool - immutable
323330
# Label slots
324-
new._labels_packed = self._labels_packed # bytes - immutable
331+
new._has_labels = self._has_labels # bool - immutable
325332
# IPVPN slots
326333
new._rd_packed = self._rd_packed # bytes - immutable
327334
return new
@@ -331,21 +338,43 @@ def has_rd(cls) -> bool:
331338
return True
332339

333340
def pack_nlri(self, negotiated: Negotiated) -> Buffer:
334-
# Wire format: [addpath?][mask][labels][rd][prefix]
335-
mask = len(self._labels_packed) * 8 + len(self._rd_packed) * 8 + self.cidr.mask
336-
packed = bytes([mask]) + self._labels_packed + self._rd_packed + self.cidr.pack_ip()
341+
"""Pack NLRI for wire transmission.
337342
338-
if not negotiated.addpath.send(self.afi, self.safi):
339-
return packed # No addpath - return directly
343+
Wire format: [addpath?][mask][labels][rd][prefix]
344+
_packed format: [addpath?][mask][labels][prefix]
340345
341-
# ADD-PATH negotiated: MUST prepend 4-byte path ID
342-
if self.path_info is PathInfo.DISABLED:
343-
addpath = PathInfo.NOPATH.pack_path()
346+
Must recalculate mask to include RD bits and insert RD after labels.
347+
"""
348+
base = self._mask_offset
349+
stored_mask = self._packed[base]
350+
label_end = self._label_end_offset
351+
352+
# Get labels and prefix bytes from _packed
353+
labels_bytes = self._packed[base + 1 : label_end]
354+
prefix_bytes = self._packed[label_end:]
355+
356+
# Recalculate mask: stored_mask + RD bits
357+
rd_bits = len(self._rd_packed) * 8
358+
combined_mask = stored_mask + rd_bits
359+
360+
# Build wire NLRI: [mask][labels][rd][prefix]
361+
nlri = bytes([combined_mask]) + labels_bytes + self._rd_packed + prefix_bytes
362+
363+
send_addpath = negotiated.addpath.send(self.afi, self.safi)
364+
if send_addpath:
365+
if self._has_addpath:
366+
# Return with stored addpath bytes
367+
return self._packed[:PATH_INFO_SIZE] + nlri
368+
# Need to prepend NOPATH
369+
return bytes(PathInfo.NOPATH.pack_path()) + nlri
344370
else:
345-
addpath = self.path_info.pack_path()
346-
return bytes(addpath) + packed
371+
return nlri # No addpath
347372

348373
def index(self) -> bytes:
374+
"""Generate unique index for RIB lookup.
375+
376+
Index uses RD + prefix (without labels) for uniqueness.
377+
"""
349378
if self.path_info is PathInfo.NOPATH:
350379
addpath = b'no-pi'
351380
elif self.path_info is PathInfo.DISABLED:
@@ -369,7 +398,8 @@ def unpack_nlri(
369398
"""Unpack IPVPN NLRI from wire format.
370399
371400
Uses SAFI to determine RD presence (exact, not heuristic).
372-
Wire format: [mask][labels][rd][prefix]
401+
Wire format: [addpath?][mask][labels][rd][prefix]
402+
Storage: _packed = [addpath?][mask][labels][prefix], _rd_packed = rd
373403
"""
374404
from struct import unpack
375405

@@ -382,28 +412,34 @@ def unpack_nlri(
382412
else:
383413
path_info = PathInfo.DISABLED
384414

385-
mask = data[0]
415+
original_mask = data[0]
386416
data = data[1:]
387417

388418
# Get RD size from Family.size (exact, not heuristic)
389419
_, rd_size = Family.size.get((afi, safi), (0, 0))
390420
rd_bits = rd_size * 8
391421

422+
# Track consumed labels for storage
423+
labels_bytes_list: list[bytes] = []
424+
mask = original_mask
425+
392426
# Parse labels using mask (original algorithm from INET.unpack_nlri)
393-
labels_list: list[int] = []
394427
if safi.has_label():
395428
while mask - rd_bits >= LABEL_SIZE_BITS:
396-
label = int(unpack('!L', bytes([0]) + bytes(data[:3]))[0])
429+
label_chunk = bytes(data[:3])
430+
label = int(unpack('!L', bytes([0]) + label_chunk)[0])
431+
labels_bytes_list.append(label_chunk)
397432
data = data[3:]
398433
mask -= LABEL_SIZE_BITS
399-
labels_list.append(label >> 4)
400434
if label == LABEL_WITHDRAW_VALUE and action == Action.WITHDRAW:
401435
break
402436
if label == LABEL_NEXTHOP_VALUE:
403437
break
404438
if label & LABEL_BOTTOM_OF_STACK_BIT:
405439
break
406440

441+
labels_packed = b''.join(labels_bytes_list)
442+
407443
# Parse RD if present (exact from SAFI, not heuristic)
408444
rd_packed = b''
409445
if rd_size:
@@ -424,21 +460,24 @@ def unpack_nlri(
424460

425461
network, data = data[:size], data[size:]
426462

427-
# Build wire format: [addpath:4?][mask:1][prefix:var]
428-
cidr_packed = bytes([mask]) + bytes(network)
463+
# Build _packed format: [addpath:4?][mask:1][labels:3n][prefix:var]
464+
# The mask stored is labels + prefix (without RD bits)
465+
stored_mask = len(labels_packed) * 8 + mask
466+
nlri_packed = bytes([stored_mask]) + labels_packed + bytes(network)
467+
429468
has_addpath = path_info is not PathInfo.DISABLED
430469
if has_addpath:
431-
packed = bytes(path_info.pack_path()) + cidr_packed
470+
packed = bytes(path_info.pack_path()) + nlri_packed
432471
else:
433-
packed = cidr_packed
472+
packed = nlri_packed
434473

435474
# Create NLRI
436475
instance = object.__new__(cls)
437476
NLRI.__init__(instance, afi, safi, action)
438477
instance._packed = packed
439478
instance._has_addpath = has_addpath
479+
instance._has_labels = len(labels_packed) > 0
440480
instance.nexthop = IP.NoNextHop
441-
instance._labels_packed = Labels.make_labels(labels_list).pack_labels() if labels_list else b''
442481
instance._rd_packed = rd_packed
443482

444483
return instance, data

0 commit comments

Comments
 (0)