Skip to content

Commit eebb645

Browse files
thomas-manginclaude
andcommitted
refactor: Store Buffer directly in attribute _packed fields
Remove unnecessary bytes() conversions throughout attribute classes. The _packed field now stores Buffer (bytes or memoryview) directly, avoiding copies when data comes from memoryview slices. Changes: - Update __init__ signatures to accept Buffer instead of bytes - Remove data_bytes = bytes(data) patterns in from_packet() methods - Use data directly for length checks and comparisons - Update tests to use factory methods instead of raw constructors 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent 9cc6350 commit eebb645

34 files changed

+387
-424
lines changed

src/exabgp/bgp/message/open/asn.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,12 @@
77

88
from __future__ import annotations
99

10+
from collections.abc import Buffer
1011
from struct import pack, unpack
1112
from typing import Type
1213

1314
from exabgp.protocol.resource import Resource
1415

15-
1616
# =================================================================== ASN
1717

1818

@@ -36,7 +36,7 @@ def pack_asn(self, asn4: bool) -> bytes:
3636
return pack('!L' if asn4 else '!H', self)
3737

3838
@classmethod
39-
def unpack_asn(cls: Type[ASN], data: bytes, klass: Type[ASN]) -> ASN:
39+
def unpack_asn(cls: Type[ASN], data: Buffer, klass: Type[ASN]) -> ASN:
4040
kls = klass
4141
if len(data) == cls.SIZE_4BYTE:
4242
value = unpack('!L', data)[0]

src/exabgp/bgp/message/open/routerid.py

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

88
from __future__ import annotations
9+
10+
from collections.abc import Buffer
911
from typing import Type
1012

11-
from exabgp.protocol.family import AFI
1213
from exabgp.protocol.ip import IPv4
1314

1415
# ===================================================================== RouterID
1516
#
1617

1718

1819
class RouterID(IPv4):
19-
def __init__(self, ip: str, packed: bytes | None = None) -> None:
20-
if IPv4.toafi(ip) != AFI.ipv4:
21-
raise ValueError('wrong address family')
22-
IPv4.__init__(self, ip, packed)
20+
def __init__(self, ip: str) -> None:
21+
IPv4.__init__(self, IPv4.pton(ip))
2322

2423
@classmethod
25-
def unpack_routerid(cls: Type[RouterID], data: bytes) -> RouterID: # pylint: disable=W0221
26-
return cls('.'.join(str(_) for _ in data), data)
24+
def unpack_routerid(cls: Type[RouterID], data: Buffer) -> RouterID:
25+
return cls(IPv4.ntop(data))

src/exabgp/bgp/message/update/__init__.py

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919
from exabgp.bgp.message.notification import Notify
2020
from exabgp.bgp.message.update.attribute import MPRNLRI, MPURNLRI, Attribute, AttributeCollection
2121
from exabgp.bgp.message.update.eor import EOR
22-
from exabgp.bgp.message.update.nlri import NLRI, NLRICollection, MPNLRICollection
22+
from exabgp.bgp.message.update.nlri import NLRI, MPNLRICollection, NLRICollection
2323
from exabgp.logger import lazyformat, lazymsg, log
2424
from exabgp.protocol.family import AFI, SAFI
2525
from exabgp.protocol.ip import IP
@@ -650,17 +650,16 @@ def unpack_message(cls, data: Buffer, negotiated: Negotiated) -> 'UpdateCollecti
650650
This method is kept for backward compatibility.
651651
"""
652652
# Convert to bytes for comparison and downstream parsing
653-
data_bytes = bytes(data) if isinstance(data, memoryview) else data
654-
length = len(data_bytes)
653+
length = len(data)
655654

656655
# Check for End-of-RIB markers (fast path)
657-
if length == EOR_IPV4_UNICAST_LENGTH and data_bytes == b'\x00\x00\x00\x00':
656+
if length == EOR_IPV4_UNICAST_LENGTH and data == b'\x00\x00\x00\x00':
658657
return EOR(AFI.ipv4, SAFI.unicast)
659-
if length == EOR_WITH_PREFIX_LENGTH and data_bytes.startswith(EOR.NLRI.PREFIX):
660-
return EOR.unpack_message(data_bytes, negotiated)
658+
if length == EOR_WITH_PREFIX_LENGTH and data.startswith(EOR.NLRI.PREFIX):
659+
return EOR.unpack_message(data, negotiated)
661660

662661
# Parse normally
663-
return cls._parse_payload(data_bytes, negotiated)
662+
return cls._parse_payload(data, negotiated)
664663

665664

666665
# Backward compatibility aliases

src/exabgp/bgp/message/update/attribute/aggregator.py

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -67,11 +67,10 @@ def from_packet(cls, data: Buffer, asn4: bool) -> 'Aggregator':
6767
Raises:
6868
ValueError: If data length is invalid
6969
"""
70-
data_bytes = bytes(data)
7170
expected_len = 8 if asn4 else 6
72-
if len(data_bytes) != expected_len:
73-
raise ValueError(f'Aggregator must be {expected_len} bytes for asn4={asn4}, got {len(data_bytes)}')
74-
return cls(data_bytes, asn4)
71+
if len(data) != expected_len:
72+
raise ValueError(f'Aggregator must be {expected_len} bytes for asn4={asn4}, got {len(data)}')
73+
return cls(data, asn4)
7574

7675
@classmethod
7776
def make_aggregator(cls, asn: ASN, speaker: IPv4) -> 'Aggregator':
@@ -181,10 +180,9 @@ def from_packet(cls, data: Buffer, asn4: bool = True) -> 'Aggregator4':
181180
182181
AS4_AGGREGATOR always uses 4-byte ASNs.
183182
"""
184-
data_bytes = bytes(data)
185-
if len(data_bytes) != 8:
186-
raise ValueError(f'AS4_AGGREGATOR must be 8 bytes, got {len(data_bytes)}')
187-
return cls(data_bytes)
183+
if len(data) != 8:
184+
raise ValueError(f'AS4_AGGREGATOR must be 8 bytes, got {len(data)}')
185+
return cls(data)
188186

189187
@classmethod
190188
def make_aggregator(cls, asn: ASN, speaker: IPv4) -> 'Aggregator4':

src/exabgp/bgp/message/update/attribute/aigp.py

Lines changed: 10 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -8,16 +8,14 @@
88
from __future__ import annotations
99

1010
from collections.abc import Buffer
11-
from struct import pack
12-
from struct import unpack
11+
from struct import pack, unpack
1312
from typing import TYPE_CHECKING, ClassVar
1413

1514
if TYPE_CHECKING:
1615
from exabgp.bgp.message.open.capability.negotiated import Negotiated
1716

1817
from exabgp.bgp.message.update.attribute.attribute import Attribute
1918

20-
2119
# ========================================================================== TLV
2220
#
2321

@@ -53,7 +51,7 @@ class AIGP(Attribute):
5351
_TLV_HEADER: ClassVar[bytes] = b'\x01\x00\x0b'
5452
_TLV_LENGTH: ClassVar[int] = 11
5553

56-
def __init__(self, packed: bytes) -> None:
54+
def __init__(self, packed: Buffer) -> None:
5755
"""Initialize AIGP from packed TLV bytes.
5856
5957
NO validation - trusted internal use only.
@@ -62,7 +60,7 @@ def __init__(self, packed: bytes) -> None:
6260
Args:
6361
packed: Raw TLV bytes (11 bytes: 1 type + 2 length + 8 value)
6462
"""
65-
self._packed: bytes = packed
63+
self._packed: Buffer = packed
6664

6765
@classmethod
6866
def from_packet(cls, data: Buffer) -> 'AIGP':
@@ -77,18 +75,17 @@ def from_packet(cls, data: Buffer) -> 'AIGP':
7775
Raises:
7876
ValueError: If data is malformed
7977
"""
80-
data_bytes = bytes(data)
81-
if len(data_bytes) < 3:
82-
raise ValueError(f'AIGP TLV too short: {len(data_bytes)} bytes')
83-
tlv_type = data_bytes[0]
84-
tlv_length = unpack('!H', data_bytes[1:3])[0]
78+
if len(data) < 3:
79+
raise ValueError(f'AIGP TLV too short: {len(data)} bytes')
80+
tlv_type = data[0]
81+
tlv_length = unpack('!H', data[1:3])[0]
8582
if tlv_type != 1:
8683
raise ValueError(f'Unknown AIGP TLV type: {tlv_type}')
8784
if tlv_length != cls._TLV_LENGTH:
8885
raise ValueError(f'Invalid AIGP TLV length: {tlv_length}')
89-
if len(data_bytes) < tlv_length:
90-
raise ValueError(f'AIGP data truncated: got {len(data_bytes)}, expected {tlv_length}')
91-
return cls(data_bytes[:tlv_length])
86+
if len(data) < tlv_length:
87+
raise ValueError(f'AIGP data truncated: got {len(data)}, expected {tlv_length}')
88+
return cls(data[:tlv_length])
9289

9390
@classmethod
9491
def from_int(cls, value: int) -> 'AIGP':

src/exabgp/bgp/message/update/attribute/aspath.py

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ class ASPath(Attribute):
8888
CONFED_SET.ID: CONFED_SET,
8989
}
9090

91-
def __init__(self, packed: bytes, asn4: bool = False) -> None:
91+
def __init__(self, packed: Buffer, asn4: bool = False) -> None:
9292
"""Initialize from packed wire-format bytes.
9393
9494
NO validation - trusted internal use only.
@@ -98,7 +98,7 @@ def __init__(self, packed: bytes, asn4: bool = False) -> None:
9898
packed: Raw attribute value bytes
9999
asn4: True if packed uses 4-byte ASNs, False for 2-byte
100100
"""
101-
self._packed: bytes = packed
101+
self._packed: Buffer = packed
102102
self._asn4: bool = asn4
103103

104104
@classmethod
@@ -115,10 +115,9 @@ def from_packet(cls, data: Buffer, asn4: bool) -> 'ASPath':
115115
Raises:
116116
Notify: If data format is invalid
117117
"""
118-
data_bytes = bytes(data)
119118
# Validate by attempting to parse - will raise Notify on error
120-
cls._unpack_segments_static(data_bytes, asn4)
121-
return cls(data_bytes, asn4)
119+
cls._unpack_segments_static(data, asn4)
120+
return cls(data, asn4)
122121

123122
@classmethod
124123
def make_aspath(
@@ -142,7 +141,7 @@ def aspath(self) -> tuple[SET | SEQUENCE | CONFED_SEQUENCE | CONFED_SET, ...]:
142141
return self._unpack_segments_static(self._packed, self._asn4)
143142

144143
@property
145-
def index(self) -> bytes:
144+
def index(self) -> Buffer:
146145
"""Get the original packed data, used for indexing/caching."""
147146
return self._packed
148147

@@ -201,7 +200,7 @@ def json(self) -> str:
201200

202201
@classmethod
203202
def _unpack_segments_static(
204-
cls, data: bytes, asn4: bool
203+
cls, data: Buffer, asn4: bool
205204
) -> tuple[SET | SEQUENCE | CONFED_SEQUENCE | CONFED_SET, ...]:
206205
"""Unpack segments from wire format."""
207206
unpacker = '!L' if asn4 else '!H'
@@ -333,10 +332,9 @@ def from_packet(cls, data: Buffer, asn4: bool = True) -> 'AS4Path':
333332
334333
AS4Path always uses 4-byte ASNs.
335334
"""
336-
data_bytes = bytes(data)
337335
# Validate by attempting to parse - will raise Notify on error
338-
cls._unpack_segments_static(data_bytes, asn4=True)
339-
return cls(data_bytes)
336+
cls._unpack_segments_static(data, asn4=True)
337+
return cls(data)
340338

341339
@classmethod
342340
def make_aspath(

src/exabgp/bgp/message/update/attribute/atomicaggregate.py

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515

1616
from exabgp.bgp.message.update.attribute.attribute import Attribute
1717

18-
1918
# ========================================================== AtomicAggregate (6)
2019
#
2120

@@ -28,7 +27,7 @@ class AtomicAggregate(Attribute):
2827
DISCARD = True
2928
VALID_ZERO = True
3029

31-
def __init__(self, packed: bytes = b'') -> None:
30+
def __init__(self, packed: Buffer) -> None:
3231
"""Initialize AtomicAggregate from packed wire-format bytes.
3332
3433
NO validation - trusted internal use only.
@@ -37,7 +36,7 @@ def __init__(self, packed: bytes = b'') -> None:
3736
Args:
3837
packed: Raw attribute value bytes (must be empty for AtomicAggregate)
3938
"""
40-
self._packed: bytes = packed
39+
self._packed: Buffer = packed
4140

4241
@classmethod
4342
def from_packet(cls, data: Buffer) -> 'AtomicAggregate':
@@ -52,10 +51,9 @@ def from_packet(cls, data: Buffer) -> 'AtomicAggregate':
5251
Raises:
5352
ValueError: If data is not empty
5453
"""
55-
data_bytes = bytes(data)
56-
if data_bytes:
57-
raise ValueError(f'AtomicAggregate must be empty, got {len(data_bytes)} bytes')
58-
return cls(data_bytes)
54+
if data:
55+
raise ValueError(f'AtomicAggregate must be empty, got {len(data)} bytes')
56+
return cls(data)
5957

6058
@classmethod
6159
def make_atomic_aggregate(cls) -> 'AtomicAggregate':

src/exabgp/bgp/message/update/attribute/attribute.py

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515
from exabgp.bgp.message.open.capability.negotiated import Negotiated
1616

1717
from exabgp.bgp.message.notification import Notify
18-
1918
from exabgp.util.cache import Cache
2019

2120
# Attribute length encoding constants
@@ -220,7 +219,7 @@ def matches(self, value: int) -> bool:
220219
# ---------------------------------------------------------------------------
221220

222221
@classmethod
223-
def _attribute(klass, value: bytes) -> bytes:
222+
def _attribute(klass, value: Buffer) -> bytes:
224223
flag: int = klass.FLAG
225224
if flag & Attribute.Flag.OPTIONAL and not value:
226225
return b''
@@ -304,16 +303,15 @@ def unpack(cls, attribute_id: int, flag: int, data: Buffer, negotiated: Negotiat
304303
cache: bool = cls.caching and cls.CACHING
305304

306305
# Convert to bytes for cache lookup (memoryview isn't hashable)
307-
data_bytes = bytes(data)
308-
if cache and data_bytes in cls.cache.get(cls.ID, {}):
309-
return cls.cache[cls.ID].retrieve(data_bytes) # type: ignore[no-any-return]
306+
if cache and data in cls.cache.get(cls.ID, {}):
307+
return cls.cache[cls.ID].retrieve(data) # type: ignore[no-any-return]
310308

311309
key: tuple[int, int] = (attribute_id, flag | Attribute.Flag.EXTENDED_LENGTH)
312310
if key in Attribute.registered_attributes.keys():
313311
instance: Attribute = cls.klass(attribute_id, flag).unpack_attribute(data, negotiated) # type: ignore[attr-defined]
314312

315313
if cache:
316-
cls.cache[cls.ID].cache(data_bytes, instance)
314+
cls.cache[cls.ID].cache(data, instance)
317315
return instance
318316

319317
raise Notify(2, 4, 'can not handle attribute id {}'.format(attribute_id))

src/exabgp/bgp/message/update/attribute/clusterlist.py

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -13,18 +13,15 @@
1313
if TYPE_CHECKING:
1414
from exabgp.bgp.message.open.capability.negotiated import Negotiated
1515

16-
from exabgp.protocol.ip import IPv4
17-
1816
from exabgp.bgp.message.update.attribute.attribute import Attribute
19-
17+
from exabgp.protocol.ip import IPv4
2018

2119
# ===================================================================
2220
#
2321

2422

2523
class ClusterID(IPv4):
26-
def __init__(self, ip: str) -> None:
27-
IPv4.__init__(self, ip)
24+
pass
2825

2926

3027
@Attribute.register()
@@ -38,7 +35,7 @@ class ClusterList(Attribute):
3835
FLAG: ClassVar[int] = Attribute.Flag.OPTIONAL
3936
CACHING: ClassVar[bool] = True
4037

41-
def __init__(self, packed: bytes) -> None:
38+
def __init__(self, packed: Buffer) -> None:
4239
"""Initialize from packed wire-format bytes.
4340
4441
NO validation - trusted internal use only.
@@ -47,7 +44,7 @@ def __init__(self, packed: bytes) -> None:
4744
Args:
4845
packed: Raw cluster list bytes (concatenated 4-byte IPv4 addresses)
4946
"""
50-
self._packed: bytes = packed
47+
self._packed: Buffer = packed
5148

5249
@classmethod
5350
def from_packet(cls, data: Buffer) -> 'ClusterList':
@@ -62,10 +59,9 @@ def from_packet(cls, data: Buffer) -> 'ClusterList':
6259
Raises:
6360
ValueError: If data length is not a multiple of 4
6461
"""
65-
data_bytes = bytes(data)
66-
if len(data_bytes) % 4 != 0:
67-
raise ValueError(f'ClusterList must be a multiple of 4 bytes, got {len(data_bytes)}')
68-
return cls(data_bytes)
62+
if len(data) % 4 != 0:
63+
raise ValueError(f'ClusterList must be a multiple of 4 bytes, got {len(data)}')
64+
return cls(data)
6965

7066
@classmethod
7167
def make_clusterlist(cls, clusters: Sequence[IPv4]) -> 'ClusterList':

0 commit comments

Comments
 (0)