Skip to content

Commit 6449739

Browse files
thomas-manginclaude
andcommitted
feat: Add attributes-only UPDATE support for round-trip testing
Add support for generating and parsing UPDATE messages that contain only path attributes with no NLRI (no announce, no withdraw). This enables round-trip testing of empty UPDATEs like those containing just ORIGIN and LOCAL_PREFERENCE. Changes: - Add Empty NLRI class for attributes-only UPDATE generation - Update attributes parser to handle commands without nlri keyword - Update UpdateCollection to generate attributes-only UPDATEs - Add decoder support for generating 'attributes' command - Update test framework to handle attributes-only encoding - Fix runtime import of Negotiated in NLRI __hash__ and index methods The new 'attributes' command syntax: attributes origin igp local-preference 100 Coverage improved from 98.0% (346/353) to 98.3% (347/353). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
1 parent 3d13d3c commit 6449739

File tree

9 files changed

+216
-15
lines changed

9 files changed

+216
-15
lines changed

.claude/exabgp/FLOWSPEC_ROUNDTRIP_LIMITATIONS.md

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

33
This document explains why certain BGP messages cannot complete a round-trip test (decode → encode → compare). The test framework marks these with `# No cmd:` comments to skip verification.
44

5-
**Current coverage:** 346/353 (98.0%) - 7 skipped
5+
**Current coverage:** 347/353 (98.3%) - 6 skipped
66

77
---
88

@@ -70,9 +70,9 @@ The self-check test now uses `generic=True` to enable round-trip for all generic
7070

7171
---
7272

73-
## 4. Empty UPDATE (1 case)
73+
## 4. Empty UPDATE (RESOLVED)
7474

75-
### The Problem
75+
### The Problem (was 1 case)
7676

7777
An UPDATE message with only path attributes and no NLRI (no announce, no withdraw).
7878

@@ -82,9 +82,15 @@ An UPDATE message with only path attributes and no NLRI (no announce, no withdra
8282

8383
### Resolution
8484

85-
**Status:** ✅ Expected (no NLRI to encode)
85+
**Status:** ✅ Fixed with `attributes` command
86+
87+
The decoder now generates `attributes` command for empty UPDATEs:
88+
89+
```
90+
attributes origin igp local-preference 100
91+
```
8692

87-
Nothing to announce or withdraw - decoder cannot generate a meaningful command.
93+
The encoder uses an Empty NLRI internally to generate the attributes-only UPDATE.
8894

8995
---
9096

@@ -135,17 +141,18 @@ Supported for all families: IPv4/IPv6, FlowSpec, MCAST-VPN, MUP, VPLS.
135141
| Interface-set transitive | 0 || Fixed with `transitive` JSON field |
136142
| Withdraw with attrs | 6 || RFC normalization (correct) |
137143
| Partial-decode attrs | 0 || Fixed with `--generic` decode mode |
138-
| Empty UPDATE | 1 || No NLRI to encode |
144+
| Empty UPDATE | 0 || Fixed with `attributes` command |
139145
| Multi-NLRI batching | 0 || Fixed with `group` command |
140146
| Pure generic attrs | 0 || Fixed with `attribute [...]` |
141-
| **Total skipped** | **7** | | |
147+
| **Total skipped** | **6** | | |
142148

143149
---
144150

145151
## Coverage History
146152

147153
| Date | Passed | Skipped | Coverage |
148154
|------|--------|---------|----------|
155+
| 2025-12-10 | 347 | 6 | 98.3% |
149156
| 2025-12-10 | 346 | 7 | 98.0% |
150157
| 2025-12-10 | 345 | 8 | 97.7% |
151158
| 2025-12-10 | 332 | 17 | 95.1% |

qa/api/api-flow.ci

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
option:file:api-flow.conf
22
1:cmd:announce eor ipv4 flow
33
1:raw:FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF:001E:02:00000007900F0003000185
4-
# No cmd: empty UPDATE (attributes only, no NLRI to encode)
4+
1:cmd:attributes origin igp local-preference 100
55
1:raw:FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF:0025:02:0000000E4001010040020040050400000064
66
1:json:{"exabgp":"6.0.0","type":"update","neighbor":{"address":{"local":"127.0.0.1","peer":"127.0.0.1"},"asn":{"local":1,"peer":1},"direction":"in","message":{"update":{"attribute":{"origin":"igp","local-preference":100}}}}}
77
1:cmd:withdraw ipv4 flow destination-ipv4 0.0.0.0/32 source-ipv4 0.0.0.0/32 protocol =tcp destination-port =3128 next-hop 0.0.0.0

qa/bin/test_api_encode

Lines changed: 49 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -412,15 +412,62 @@ def encode_api_command(
412412
return packed.hex().upper()
413413

414414
# Handle 'attributes' syntax: announce attributes ... nlri X Y Z
415-
if parts[1] == "attributes":
415+
# OR attributes-only: attributes origin igp local-preference 100
416+
if parts[1] == "attributes" or parts[0] == "attributes":
416417
# Parse: announce attributes [path-info X] next-hop Y ... nlri A B C
417-
attr_parts = parts[2:]
418+
# OR: attributes origin igp local-preference 100 (no nlri keyword - attributes-only UPDATE)
419+
if parts[0] == "attributes":
420+
# attributes-only command: attributes origin igp local-preference 100
421+
attr_parts = parts[1:]
422+
else:
423+
attr_parts = parts[2:]
418424
nlri_idx = None
419425
for i, p in enumerate(attr_parts):
420426
if p == "nlri":
421427
nlri_idx = i
422428
break
423429
if nlri_idx is None:
430+
# Attributes-only UPDATE (no NLRI)
431+
attr_str = " ".join(attr_parts)
432+
433+
# Use unique neighbor address to avoid RIB cache collision
434+
neighbor_ip = f"127.0.{(_encode_counter >> 8) & 255}.{_encode_counter & 255}"
435+
436+
conf = f"""
437+
neighbor {neighbor_ip} {{
438+
router-id 10.0.0.2;
439+
local-address 127.0.0.1;
440+
local-as {local_as};
441+
peer-as {peer_as};
442+
family {{ ipv4 unicast; }}
443+
static {{ attributes {attr_str}; }}
444+
}}
445+
"""
446+
configuration = Configuration([conf], text=True)
447+
if not configuration.reload():
448+
return None
449+
450+
if not configuration.neighbors:
451+
return None
452+
453+
for name in configuration.neighbors.keys():
454+
neighbor = configuration.neighbors[name]
455+
_, negotiated_out = _negotiated(neighbor)
456+
457+
if not neighbor.rib.enabled:
458+
continue
459+
460+
for _ in neighbor.rib.outgoing.updates(False):
461+
pass
462+
463+
for route in neighbor.rib.outgoing.cached_routes():
464+
for packed in UpdateCollection(
465+
[RoutedNLRI(route.nlri, route.nexthop)], [], route.attributes
466+
).messages(negotiated_out):
467+
neighbor.rib.uncache()
468+
return packed.hex().upper()
469+
470+
neighbor.rib.uncache()
424471
return None
425472

426473
attr_str = " ".join(attr_parts[:nlri_idx])

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

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,9 @@ def split(data: Buffer) -> tuple[Buffer, Buffer, Buffer]:
205205

206206
# The routes MUST have the same attributes ...
207207
def messages(self, negotiated: Negotiated, include_withdraw: bool = True) -> Generator[bytes, None, None]:
208+
# Import here to avoid circular import
209+
from exabgp.bgp.message.update.nlri.empty import Empty
210+
208211
# Sort and classify NLRIs into IPv4 and MP categories
209212
# v4_announces/v4_withdraws store bare NLRIs (nexthop is in NEXT_HOP attribute for IPv4)
210213
# mp_announces stores RoutedNLRI by family (nexthop needed for MP_REACH_NLRI encoding)
@@ -214,11 +217,20 @@ def messages(self, negotiated: Negotiated, include_withdraw: bool = True) -> Gen
214217
mp_announces: dict[tuple[AFI, SAFI], list[RoutedNLRI]] = {}
215218
mp_withdraws: dict[tuple[AFI, SAFI], list[NLRI]] = {}
216219

220+
# Track if we have Empty NLRI (attributes-only UPDATE)
221+
has_empty_nlri = False
222+
217223
# Process announces - self._announces contains RoutedNLRI
218224
# Sort by nlri for deterministic ordering
219225
for routed in sorted(self._announces, key=lambda r: r.nlri):
220226
nlri = routed.nlri
221227
nexthop = routed.nexthop
228+
229+
# Skip Empty NLRI but remember we had one
230+
if isinstance(nlri, Empty):
231+
has_empty_nlri = True
232+
continue
233+
222234
if nlri.family().afi_safi() not in negotiated.families:
223235
continue
224236

@@ -242,6 +254,11 @@ def messages(self, negotiated: Negotiated, include_withdraw: bool = True) -> Gen
242254

243255
# Process withdraws - bare NLRIs (no nexthop needed)
244256
for nlri in sorted(self._withdraws):
257+
# Skip Empty NLRI in withdraws
258+
if isinstance(nlri, Empty):
259+
has_empty_nlri = True
260+
continue
261+
245262
if nlri.family().afi_safi() not in negotiated.families:
246263
continue
247264

@@ -259,6 +276,11 @@ def messages(self, negotiated: Negotiated, include_withdraw: bool = True) -> Gen
259276
has_v4 = v4_announces or v4_withdraws
260277
has_mp = mp_announces or mp_withdraws
261278
if not has_v4 and not has_mp:
279+
# Attributes-only UPDATE (Empty NLRI case)
280+
if has_empty_nlri and self._attributes:
281+
attr = self.attributes.pack_attribute(negotiated, with_default=True)
282+
# Generate UPDATE with no withdrawn routes and no NLRI, just attributes
283+
yield self._message(UpdateCollection.prefix(b'') + UpdateCollection.prefix(attr))
262284
return
263285

264286
# If all we have is MP_UNREACH_NLRI, we do not need the default

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@
5151
from exabgp.bgp.message.update.nlri.bgpls import BGPLS
5252
from exabgp.bgp.message.update.nlri.mup import MUP
5353
from exabgp.bgp.message.update.nlri.mvpn import MVPN
54+
from exabgp.bgp.message.update.nlri.empty import Empty
5455
from exabgp.bgp.message.update.nlri.collection import NLRICollection, MPNLRICollection
5556

5657
__all__ = [
@@ -66,6 +67,7 @@
6667
'BGPLS',
6768
'MUP',
6869
'MVPN',
70+
'Empty',
6971
'NLRICollection',
7072
'MPNLRICollection',
7173
]
Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
"""empty.py
2+
3+
Empty NLRI for attributes-only UPDATE messages.
4+
5+
Created for API command round-trip testing (empty UPDATE with just attributes).
6+
Copyright (c) 2024 Exa Networks. All rights reserved.
7+
License: 3-clause BSD. (See the COPYRIGHT file)
8+
"""
9+
10+
from __future__ import annotations
11+
12+
from typing import TYPE_CHECKING, Any
13+
14+
if TYPE_CHECKING:
15+
from exabgp.bgp.message.open.capability.negotiated import Negotiated
16+
17+
from exabgp.bgp.message import Action
18+
from exabgp.bgp.message.update.nlri.nlri import NLRI
19+
from exabgp.bgp.message.update.nlri.qualifier.path import PathInfo
20+
from exabgp.protocol.family import AFI, SAFI
21+
from exabgp.util.types import Buffer
22+
23+
24+
class Empty(NLRI):
25+
"""Empty NLRI for attributes-only UPDATE messages.
26+
27+
This NLRI type represents "no routes" and is used to generate
28+
UPDATE messages that contain only path attributes (no NLRI).
29+
30+
Such messages are valid per RFC 4271 but don't announce or withdraw
31+
any routes - they're primarily useful for testing.
32+
"""
33+
34+
__slots__ = ()
35+
36+
# Class attribute to identify Empty NLRI instances
37+
EMPTY_NLRI = True
38+
39+
def __init__(self, afi: AFI = AFI.ipv4, safi: SAFI = SAFI.unicast, action: Action = Action.ANNOUNCE) -> None:
40+
"""Create an Empty NLRI.
41+
42+
Args:
43+
afi: Address Family Identifier (default: ipv4)
44+
safi: Subsequent Address Family Identifier (default: unicast)
45+
action: Action (ANNOUNCE or WITHDRAW, default: ANNOUNCE)
46+
"""
47+
super().__init__(afi, safi, action, PathInfo.DISABLED)
48+
self._packed = b''
49+
50+
def pack_nlri(self, negotiated: 'Negotiated') -> Buffer:
51+
"""Pack to empty bytes (no NLRI)."""
52+
return b''
53+
54+
def json(self, compact: bool = False) -> str:
55+
"""JSON representation (empty object)."""
56+
return '{}'
57+
58+
def __str__(self) -> str:
59+
return 'empty'
60+
61+
def __repr__(self) -> str:
62+
return 'Empty()'
63+
64+
def __copy__(self) -> 'Empty':
65+
new = object.__new__(Empty)
66+
self._copy_nlri_slots(new)
67+
return new
68+
69+
def __deepcopy__(self, memo: dict[Any, Any]) -> 'Empty':
70+
new = object.__new__(Empty)
71+
self._deepcopy_nlri_slots(new, memo)
72+
return new
73+
74+
def feedback(self, action: Action) -> str:
75+
return 'empty'

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,8 @@ def __deepcopy__(self, memo: dict[Any, Any]) -> 'NLRI':
137137
raise NotImplementedError(f'{type(self).__name__} must implement __deepcopy__')
138138

139139
def __hash__(self) -> int:
140+
from exabgp.bgp.message.open.capability.negotiated import Negotiated
141+
140142
return hash('{}:{}:{}'.format(self.afi, self.safi, self.pack_nlri(Negotiated.UNSET).hex()))
141143

142144
def __eq__(self, other: Any) -> bool:
@@ -168,6 +170,8 @@ def add(self, data: Any) -> bool:
168170
raise NotImplementedError('add() only implemented by Flow NLRI')
169171

170172
def index(self) -> bytes:
173+
from exabgp.bgp.message.open.capability.negotiated import Negotiated
174+
171175
return bytes(Family.index(self)) + self.pack_nlri(Negotiated.UNSET)
172176

173177
def pack_nlri(self, negotiated: Negotiated) -> Buffer:

src/exabgp/configuration/command.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -529,6 +529,12 @@ def decode_to_api_command(payload_hex: str, neighbor: 'Neighbor', generic: bool
529529
else:
530530
commands.append(f'withdraw {api_family} {nlri_info}')
531531

532+
# Attributes-only UPDATE (no announce, no withdraw, just attributes)
533+
if not commands and attributes:
534+
attr_parts = format_attributes(attributes)
535+
if attr_parts:
536+
commands.append('attributes ' + ' '.join(attr_parts))
537+
532538
return commands
533539

534540
except Exception:

src/exabgp/configuration/static/__init__.py

Lines changed: 43 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -137,19 +137,43 @@ def route(tokeniser: Any) -> list[Route]:
137137

138138
@ParseStatic.register('attributes', 'append-route')
139139
def attributes(tokeniser: Any) -> list[Route]:
140-
"""Parse attributes with multiple NLRIs using deferred construction (Settings pattern).
140+
"""Parse attributes with optional NLRIs using deferred construction (Settings pattern).
141141
142142
Collects shared settings first, then creates immutable NLRIs for each prefix.
143+
If no 'nlri' keyword or no prefixes, returns an attributes-only Route
144+
that generates an UPDATE with just path attributes and no NLRI.
143145
"""
144146
from copy import copy
145147

148+
from exabgp.bgp.message.update.nlri.empty import Empty
149+
146150
nlri_action = Action.ANNOUNCE if tokeniser.announce else Action.WITHDRAW
147-
ipmask = prefix(lambda: tokeniser.tokens[-1])
148-
tokeniser.afi = ipmask.afi
151+
152+
# Check if there are any IP prefixes in the tokens (look-ahead for nlri keyword)
153+
has_nlri_keyword = 'nlri' in tokeniser.tokens
154+
155+
# Try to parse the last token as a prefix to determine AFI
156+
# If it fails, this is an attributes-only command
157+
has_prefix = False
158+
ipmask = None
159+
if has_nlri_keyword:
160+
try:
161+
ipmask = prefix(lambda: tokeniser.tokens[-1])
162+
has_prefix = True
163+
except (ValueError, KeyError):
164+
pass
165+
166+
if has_prefix and ipmask is not None:
167+
tokeniser.afi = ipmask.afi
168+
else:
169+
tokeniser.afi = AFI.ipv4 # Default for attributes-only
149170

150171
# Create template settings with initial values
151172
template_settings = INETSettings()
152-
template_settings.afi = IP.toafi(ipmask.top())
173+
if has_prefix and ipmask is not None:
174+
template_settings.afi = IP.toafi(ipmask.top())
175+
else:
176+
template_settings.afi = AFI.ipv4
153177
template_settings.action = nlri_action
154178
attr = AttributeCollection()
155179

@@ -164,15 +188,24 @@ def attributes(tokeniser: Any) -> list[Route]:
164188
elif has_label:
165189
nlri_class = Label
166190
template_settings.safi = SAFI.nlri_mpls
167-
else:
191+
elif has_prefix and ipmask is not None:
168192
nlri_class = INET
169193
template_settings.safi = IP.tosafi(ipmask.top())
194+
else:
195+
nlri_class = INET
196+
template_settings.safi = SAFI.unicast
170197

171198
# Parse shared attributes - collect into template settings
172199
while True:
173200
command = tokeniser()
174201

175202
if not command:
203+
# No more tokens - check if we have NLRIs or just attributes
204+
if not has_nlri_keyword:
205+
# Attributes-only: return Route with Empty NLRI
206+
if attr:
207+
empty_nlri = Empty(AFI.ipv4, SAFI.unicast, nlri_action)
208+
return [Route(empty_nlri, attr)]
176209
return []
177210

178211
if command == 'nlri':
@@ -223,4 +256,9 @@ def attributes(tokeniser: Any) -> list[Route]:
223256
new_nlri = nlri_class.from_settings(settings)
224257
routes.append(Route(new_nlri, attr, nexthop=settings.nexthop))
225258

259+
# If 'nlri' keyword was present but no prefixes followed, return attributes-only
260+
if not routes and attr:
261+
empty_nlri = Empty(AFI.ipv4, SAFI.unicast, nlri_action)
262+
return [Route(empty_nlri, attr)]
263+
226264
return routes

0 commit comments

Comments
 (0)