Repository navigation
feat: decode enterprise-specific Information Elements (RFC 5610) - #4
Conversation
Each Decoder now keeps a table of enterprise-specific Information Element definitions, used to decode fields the IANA registry does not describe: - definitions registered by the caller (register_enterprise_element), the Collecting Process's "own internal definition", which always take precedence; - definitions learned from RFC 5610 Information Element type records, scoped to the Transport Session and Observation Domain, under the RFC's rules: they never replace registered or IANA definitions, conflicting records disable the element, invalid data type / semantics combinations (Section 3.10) and names with NUL characters are ignored, and the learned table is bounded (max_enterprise_elements). Also adds Decoder::enterprise_element, field_name and learned_element_count, DataType/DataTypeSemantics::from_value, unit and serde tests, and RFC 5610 type records in the structured fuzzer. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Claude finished @matthieugouel's task in 34s —— View job Review of PR #4: enterprise-specific Information Elements (RFC 5610)I read the The core logic looks sound:
I found no panics or unbounded allocations. The points below are in priority order. CLAUDE.md convention gapsThese come from the "Implementing a New Specification" checklist.
Behaviour worth a second look
Overall this looks good. The items above are mostly documentation and validation steps that CLAUDE.md requires. |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
- A field with the Enterprise bit set and PEN 0 (reserved) is no longer decoded with PEN 0 definitions (IANA elements newer than the registry); it is kept raw, and has no name. - Validate RFC 5610 Table 1 (data type values) and Table 4 (type record elements and Scope Fields) against the implementation in the specs tests. The type record element IDs move to the public registry::type_records module. - Reproduce the RFC 5610 Appendix A example as a unit test, with Erratum 4731. Document in SPEC_ISSUES.md that the example's Field Length of 65536 should be 65535, which the erratum missed. - Document RFC 5610 errata and the semantics-restriction deviation (lists, unsigned256), and add an exhaustive data type x semantics test. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Thanks for the review. Here's how each point was handled in the latest push: CLAUDE.md gaps
Behaviour
|
Covers the 3 patch lines reported by Codecov on #4. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Summary
Enterprise-specific (vendor) fields were always kept as
FieldValue::Raw. EachDecodernow keeps a table of enterprise element definitions, filled from two sources:Decoder::register_enterprise_element, e.g. from vendor documentation. This is the Collecting Process's "own internal definition", which RFC 5610 §3.9 says type records must never replace. Registered definitions apply to all Observation Domains and surviveclear(). Registering with PEN 0 also covers IANA elements assigned after the crate's registry snapshot.privateEnterpriseNumberandinformationElementId. They are scoped to the Transport Session and Observation Domain, and apply to the following Data Sets.The RFC's rules (all MUSTs) are enforced:
Raw)DecoderConfig::max_enterprise_elements, default 4096), and extra type records are ignored rather than failing the MessageNew API:
EnterpriseElement,ElementSource,Decoder::{register_enterprise_element, enterprise_element, field_name, learned_element_count},DataType::from_value,DataTypeSemantics::from_value.Registration is the part most users will need. None of the 13 vendor samples send type records: vendors document their elements out-of-band.
Test plan
tests/unit/enterprise_elements.rs), one per rule above, plus a serde round-tripfuzz_structurednow generates type records (including PEN 0, PEN 29305, out-of-registry types and semantics) and enterprise fields with matching PENs: 4.0M runs, no crashes-D warnings) clean; 140 tests pass including network ones; coverage 94% (99% onparsers/elements.rs)🤖 Generated with Claude Code