From a09e0312dd1257404669ddc2e6a2cf4264d072a6 Mon Sep 17 00:00:00 2001 From: Ari Sulistiono Date: Tue, 6 Oct 2026 19:00:28 +0700 Subject: [PATCH] P0: centralize VLAN invariants across Process Bus paths Use one IEEE 802.1Q wire invariant for VID/PCP validation, reject reserved VID 4095 before deployment/encoding, preserve priority-tagged VID 0 where generic Ethernet permits it, and add SV/SCL/PTP boundary regressions. Tracks #139. --- apps/arstack_studio/src/DeviceController.cpp | 6 +++++- .../main/profile_control.cpp | 8 ++++++-- .../main/runtime_profile.cpp | 6 +++++- include/ariec61850/ethernet/ethernet.hpp | 14 ++++++++++++++ .../sampled_values/publisher_profile.hpp | 7 ++++--- src/ethernet/ethernet.cpp | 6 +++--- src/sampled_values/frame_codec.cpp | 3 ++- src/time_sync/ptp.cpp | 3 ++- tests/test_ptp.cpp | 16 ++++++++++++++++ tests/test_sampled_values.cpp | 19 +++++++++++++++++++ tests/test_scl.cpp | 18 ++++++++++++++++++ 11 files changed, 94 insertions(+), 12 deletions(-) diff --git a/apps/arstack_studio/src/DeviceController.cpp b/apps/arstack_studio/src/DeviceController.cpp index ad6eb65a1..95e2c7a66 100644 --- a/apps/arstack_studio/src/DeviceController.cpp +++ b/apps/arstack_studio/src/DeviceController.cpp @@ -4,6 +4,8 @@ #include "DeviceIoWorker.hpp" +#include "ariec61850/ethernet/ethernet.hpp" + #include #include #include @@ -669,7 +671,9 @@ bool DeviceController::deployProfile(const QVariantMap& profile) { const QString idHex = utf8Hex(svId); const QString dataSetHex = includeDataSet ? utf8Hex(dataSet) : QStringLiteral("-"); if (svId.isEmpty() || idHex.isEmpty() || idHex.size() > 180 || dataSetHex.size() > 170 || - mac.size() != 12 || appId == 0U || appId > 65535U || vlanId > 4095U || pcp > 7U || + mac.size() != 12 || appId == 0U || appId > 65535U || + vlanId > ar::iec61850::ethernet::maximum_vlan_id || + pcp > ar::iec61850::ethernet::maximum_vlan_priority || rate == 0U || rate > 65535U || modulus == 0U || modulus > 65535U || noAsdu != 1U) { setError(QStringLiteral("Compiled profile exceeds the current bounded device bridge.")); return false; diff --git a/embedded/esp32p4_smv_injector/main/profile_control.cpp b/embedded/esp32p4_smv_injector/main/profile_control.cpp index 95f3decb2..3b6044a62 100644 --- a/embedded/esp32p4_smv_injector/main/profile_control.cpp +++ b/embedded/esp32p4_smv_injector/main/profile_control.cpp @@ -6,6 +6,8 @@ #include "runtime_profile.hpp" #include "smp_synch_lab.hpp" +#include "ariec61850/ethernet/ethernet.hpp" + #include "esp_log.h" #include "sdkconfig.h" @@ -311,8 +313,10 @@ void handle_profile_command(char* arguments) noexcept { if (!parse_u32(appid_text, appid) || appid == 0U || appid > 65535U || !decode_mac(mac_text, mac) || !parse_u32(vlan_present_text, vlan_present) || vlan_present > 1U || - !parse_u32(vlan_id_text, vlan_id) || vlan_id > 4095U || - !parse_u32(pcp_text, pcp) || pcp > 7U || !no_extra(&save)) { + !parse_u32(vlan_id_text, vlan_id) || + vlan_id > ar::iec61850::ethernet::maximum_vlan_id || + !parse_u32(pcp_text, pcp) || + pcp > ar::iec61850::ethernet::maximum_vlan_priority || !no_extra(&save)) { ESP_LOGE(kTag, "Invalid PROFILE L2 payload"); return; } diff --git a/embedded/esp32p4_smv_injector/main/runtime_profile.cpp b/embedded/esp32p4_smv_injector/main/runtime_profile.cpp index 2eb2dc12e..58dcc194c 100644 --- a/embedded/esp32p4_smv_injector/main/runtime_profile.cpp +++ b/embedded/esp32p4_smv_injector/main/runtime_profile.cpp @@ -2,6 +2,8 @@ #include "runtime_profile.hpp" +#include "ariec61850/ethernet/ethernet.hpp" + #include "freertos/FreeRTOS.h" #include @@ -46,7 +48,9 @@ bool runtime_profile_validate(const RuntimePublisherProfile& profile) noexcept { profile.sv_id[0] == '\0') { return false; } - if (profile.vlan_present && (profile.vlan_id > 4095U || profile.vlan_priority > 7U)) { + if (profile.vlan_present && + (!ar::iec61850::ethernet::is_valid_vlan_id(profile.vlan_id) || + !ar::iec61850::ethernet::is_valid_vlan_priority(profile.vlan_priority))) { return false; } if (profile.include_data_set && profile.data_set_reference[0] == '\0') { diff --git a/include/ariec61850/ethernet/ethernet.hpp b/include/ariec61850/ethernet/ethernet.hpp index 1d23b5ed2..dac0faa44 100644 --- a/include/ariec61850/ethernet/ethernet.hpp +++ b/include/ariec61850/ethernet/ethernet.hpp @@ -16,6 +16,20 @@ inline constexpr std::uint16_t goose_ethertype = 0x88B8; inline constexpr std::uint16_t sampled_values_ethertype = 0x88BA; inline constexpr std::uint16_t ptp_ethertype = 0x88F7; +// IEEE 802.1Q VID 4095 is reserved. VID 0 remains valid at the generic +// Ethernet layer for priority-tagged frames; profile-specific users may impose +// a narrower policy without changing the wire invariant. +inline constexpr std::uint16_t maximum_vlan_id = 4094U; +inline constexpr std::uint8_t maximum_vlan_priority = 7U; + +[[nodiscard]] constexpr bool is_valid_vlan_id(const std::uint16_t vlan_id) noexcept { + return vlan_id <= maximum_vlan_id; +} + +[[nodiscard]] constexpr bool is_valid_vlan_priority(const std::uint8_t priority) noexcept { + return priority <= maximum_vlan_priority; +} + class MacAddress final { public: MacAddress() = default; diff --git a/include/ariec61850/sampled_values/publisher_profile.hpp b/include/ariec61850/sampled_values/publisher_profile.hpp index f95597d6b..fc4f443ef 100644 --- a/include/ariec61850/sampled_values/publisher_profile.hpp +++ b/include/ariec61850/sampled_values/publisher_profile.hpp @@ -2,6 +2,7 @@ #pragma once +#include "ariec61850/ethernet/ethernet.hpp" #include "ariec61850/scl/model.hpp" #include @@ -134,10 +135,10 @@ class SvPublisherProfileCompiler final { profile.vlan_present = true; profile.vlan_id = *stream.address.vlan_id; profile.vlan_priority = *stream.address.vlan_priority; - if (profile.vlan_id > 4095U) { - result.errors.push_back("SV VLAN ID exceeds the 12-bit Ethernet VLAN range."); + if (!ethernet::is_valid_vlan_id(profile.vlan_id)) { + result.errors.push_back("SV VLAN ID must be 0..4094; VID 4095 is reserved."); } - if (profile.vlan_priority > 7U) { + if (!ethernet::is_valid_vlan_priority(profile.vlan_priority)) { result.errors.push_back("SV VLAN priority exceeds the 3-bit Ethernet PCP range."); } } diff --git a/src/ethernet/ethernet.cpp b/src/ethernet/ethernet.cpp index b0ae6db1b..0f512ad9e 100644 --- a/src/ethernet/ethernet.cpp +++ b/src/ethernet/ethernet.cpp @@ -115,10 +115,10 @@ std::string MacAddress::to_string() const { } std::uint16_t VlanTag::to_tag_control_information() const { - if (priority_code_point > 7U) { + if (!is_valid_vlan_priority(priority_code_point)) { throw std::out_of_range("VLAN priority must be 0..7."); } - if (vlan_id > 4094U) { + if (!is_valid_vlan_id(vlan_id)) { throw std::out_of_range("VLAN ID must be 0..4094."); } @@ -182,7 +182,7 @@ bool EthernetFrameCodec::try_decode(const std::span bytes, VlanTag::from_tag_control_information(read_u16_be(bytes, 14U)); // IEEE 802.1Q reserves VID 4095. Keep try_decode closed under encode: // every successfully decoded VlanTag must be valid for re-encoding. - if (decoded_vlan.vlan_id > 4094U) { + if (!is_valid_vlan_id(decoded_vlan.vlan_id)) { return false; } vlan = decoded_vlan; diff --git a/src/sampled_values/frame_codec.cpp b/src/sampled_values/frame_codec.cpp index 80f62e830..5b7ecc8fc 100644 --- a/src/sampled_values/frame_codec.cpp +++ b/src/sampled_values/frame_codec.cpp @@ -28,7 +28,8 @@ void write_u16_be( [[nodiscard]] std::optional vlan_tci( const ethernet::VlanTag& vlan) noexcept { - if (vlan.priority_code_point > 7U || vlan.vlan_id > 4094U) { + if (!ethernet::is_valid_vlan_priority(vlan.priority_code_point) || + !ethernet::is_valid_vlan_id(vlan.vlan_id)) { return std::nullopt; } const auto value = diff --git a/src/time_sync/ptp.cpp b/src/time_sync/ptp.cpp index 073d8aef6..d4d5331a0 100644 --- a/src/time_sync/ptp.cpp +++ b/src/time_sync/ptp.cpp @@ -226,7 +226,8 @@ std::vector PtpCodec::build_ethernet_frame( const std::span ptp_message, const std::optional vlan_id, const std::uint8_t vlan_priority) { - if (vlan_priority > 7U || (vlan_id.has_value() && *vlan_id > 4095U)) { + if (!ethernet::is_valid_vlan_priority(vlan_priority) || + (vlan_id.has_value() && !ethernet::is_valid_vlan_id(*vlan_id))) { return {}; } diff --git a/tests/test_ptp.cpp b/tests/test_ptp.cpp index fb6ea0a14..b9ddb5c9e 100644 --- a/tests/test_ptp.cpp +++ b/tests/test_ptp.cpp @@ -283,6 +283,21 @@ void transport_specific_change_is_visible_as_health_warning() { CHECK(resolve_smp_synch(report, true) == SmpSynchValue::local_synchronized); } +void ethernet_builder_enforces_canonical_vlan_boundaries() { + using namespace ar::iec61850::time_sync; + const std::array source{0x02U, 0x00U, 0x00U, 0x00U, 0x00U, 0x01U}; + const auto message = PtpCodec::build_sync(make_options()); + + CHECK(!PtpCodec::build_ethernet_frame( + ptp_general_multicast_mac, source, message, std::uint16_t{0U}, std::uint8_t{4U}).empty()); + CHECK(!PtpCodec::build_ethernet_frame( + ptp_general_multicast_mac, source, message, std::uint16_t{4094U}, std::uint8_t{7U}).empty()); + CHECK(PtpCodec::build_ethernet_frame( + ptp_general_multicast_mac, source, message, std::uint16_t{4095U}, std::uint8_t{4U}).empty()); + CHECK(PtpCodec::build_ethernet_frame( + ptp_general_multicast_mac, source, message, std::uint16_t{100U}, std::uint8_t{8U}).empty()); +} + void malformed_or_non_ptp_frames_are_rejected() { using namespace ar::iec61850::time_sync; PtpFrame frame; @@ -308,6 +323,7 @@ int main() { {"PTP VLAN/QinQ/transportSpecific analyzer parser", ethernet_vlan_qinq_and_transport_specific_are_parsed_for_analyzer_use}, {"PTP monitor health and smpSynch", passive_monitor_health_drives_conservative_smp_synch_policy}, {"PTP transportSpecific stability", transport_specific_change_is_visible_as_health_warning}, + {"PTP Ethernet VLAN boundaries", ethernet_builder_enforces_canonical_vlan_boundaries}, {"PTP malformed input", malformed_or_non_ptp_frames_are_rejected}, }; diff --git a/tests/test_sampled_values.cpp b/tests/test_sampled_values.cpp index f1233ec2a..9a871015a 100644 --- a/tests/test_sampled_values.cpp +++ b/tests/test_sampled_values.cpp @@ -115,6 +115,25 @@ void sampled_values_frame_round_trips_vlan_process_bus_header() { CHECK(SampledValuesFrameCodec::try_decode(encoded, decoded)); CHECK(decoded == frame); + auto priority_tagged = frame; + priority_tagged.vlan = ethernet::VlanTag{4U, 0U}; + CHECK(SampledValuesFrameCodec::encoded_size(priority_tagged).has_value()); + + auto maximum_vlan = frame; + maximum_vlan.vlan = ethernet::VlanTag{7U, 4094U}; + CHECK(SampledValuesFrameCodec::encoded_size(maximum_vlan).has_value()); + + auto reserved_vlan = frame; + reserved_vlan.vlan = ethernet::VlanTag{4U, 4095U}; + CHECK(!SampledValuesFrameCodec::encoded_size(reserved_vlan).has_value()); + bool reserved_vlan_rejected = false; + try { + static_cast(SampledValuesFrameCodec::encode(reserved_vlan)); + } catch (const std::out_of_range&) { + reserved_vlan_rejected = true; + } + CHECK(reserved_vlan_rejected); + auto wrong_ethertype = encoded; wrong_ethertype[16] = 0x88U; wrong_ethertype[17] = 0xB8U; diff --git a/tests/test_scl.cpp b/tests/test_scl.cpp index ce83efd42..0a1007f30 100644 --- a/tests/test_scl.cpp +++ b/tests/test_scl.cpp @@ -216,6 +216,24 @@ void parser_compiles_structured_4800_sv_profile_without_drift() { CHECK(profile.payload_size_bytes == 64U); CHECK(profile.asdu_options.element_present); + auto maximum_vlan_stream = stream; + maximum_vlan_stream.address.vlan_id = static_cast(4094U); + const auto maximum_vlan = SvPublisherProfileCompiler::compile(maximum_vlan_stream, context); + CHECK(maximum_vlan.ok()); + CHECK(maximum_vlan.profile.has_value()); + CHECK(maximum_vlan.profile->vlan_id == 4094U); + + auto reserved_vlan_stream = stream; + reserved_vlan_stream.address.vlan_id = static_cast(4095U); + const auto reserved_vlan = SvPublisherProfileCompiler::compile(reserved_vlan_stream, context); + CHECK(!reserved_vlan.ok()); + CHECK(!reserved_vlan.profile.has_value()); + CHECK(std::any_of( + reserved_vlan.errors.begin(), reserved_vlan.errors.end(), + [](const std::string& error) { + return error.find("0..4094") != std::string::npos; + })); + RationalTickSchedule schedule_4800{1'000'000U, 4800U}; std::uint64_t total_ticks{}; std::uint32_t intervals_208{};