|
| 1 | +# SONiC HLD: Deterministic MACsec backend selection for gearbox ports |
| 2 | + |
| 3 | +#### Rev 0.1 |
| 4 | + |
| 5 | +## Table of Content |
| 6 | +- [Revision](#revision) |
| 7 | +- [Scope](#scope) |
| 8 | + |
| 9 | +- [Overview](#overview) |
| 10 | +- [Requirements](#requirements) |
| 11 | +- [High-Level Design](#high-level-design) |
| 12 | + - [Data Model](#data-model) |
| 13 | + - [Behavior](#behavior) |
| 14 | + - [Component Changes](#component-changes) |
| 15 | +- [Configuration and management](#configuration-and-management) |
| 16 | +- [Warmboot and Fastboot Design Impact](#warmboot-and-fastboot-design-impact) |
| 17 | +- [Backward Compatibility](#backward-compatibility) |
| 18 | +- [Testing Requirements/Design](#testing-requirementsdesign) |
| 19 | + |
| 20 | +### Revision |
| 21 | + |
| 22 | +| Rev | Date | Author/s | Change Description | |
| 23 | +|:---:|:-------:|:----------------------------------------:|:-------------------| |
| 24 | +| 0.1 | 09/2025 | Senthil Krishnamurthy, Rajshekhar Biradar| Initial draft | |
| 25 | + |
| 26 | +### Overview |
| 27 | +On gearbox ports, creating MACsec on the PHY switch fails (SAI_STATUS_NOT_IMPLEMENTED) if gearbox PHY does not have the MACsec engine. This HLD introduces a platform capability flag in the gearbox config to determine, per PHY, whether MACsec is supported (applies to all ports mapped to that PHY). MACsec orchestration will: |
| 28 | +- Use PHY switch by default on gearbox ports |
| 29 | +- Use NPU/global switch only when the platform marks the PHY as not supporting MACsec |
| 30 | + |
| 31 | + |
| 32 | +## Background / Problem |
| 33 | +- PortsOrch ties gearbox ports to a PHY `switch_id`. Legacy MACsec orchestration created MACsec objects on that PHY, causing failures on platforms where PHY SAI lacks MACsec. |
| 34 | +- Effects: errors from gbsyncd (NOT_IMPLEMENTED), and potential instability in orchagent. |
| 35 | + |
| 36 | +## Scope |
| 37 | +- Platforms with external PHY/gearbox |
| 38 | +- Components: gearsyncd, swss (GearboxUtils, PortsOrch), MACsecOrch |
| 39 | +- No changes to wpa_supplicant driver behavior |
| 40 | + |
| 41 | +### Requirements |
| 42 | +- Prevent MACsec operations on unsupported PHY SAIs |
| 43 | +- Drive behavior from a single source of truth in the platform gearbox configuration |
| 44 | +- Preserve existing behavior by default (PHY MACsec unless explicitly disabled) |
| 45 | + |
| 46 | +## High-Level Design |
| 47 | + |
| 48 | +#### Data Model |
| 49 | +Add per-PHY capability in gearbox configuration: |
| 50 | +- Field name: `macsec_supported` (boolean) |
| 51 | +- Location: top-level HWSKU `gearbox_config.json` under each `phys` item |
| 52 | +- Default semantics: if field is absent, treat as `true` (assume PHY supports MACsec) to preserve existing behavior |
| 53 | + |
| 54 | +Propagation to runtime DB: |
| 55 | +- gearsyncd (GearboxParser) forwards `macsec_supported` from `phys` into APP_DB `_GEARBOX_TABLE:phy:<id>` alongside other PHY attributes |
| 56 | +- GearboxUtils loads it via `loadPhyMap()` into `gearbox_phy_t` for lookup by MACsecOrch |
| 57 | + |
| 58 | +Example snippet (HWSKU `gearbox_config.json`; note optional `macsec_supported` on PHY): |
| 59 | +```json |
| 60 | +{ |
| 61 | + "phys": [{ |
| 62 | + "phy_id": 1, |
| 63 | + "name": "phy1", |
| 64 | + "config_file": "/usr/share/sonic/hwsku/phy1_config.json", |
| 65 | + "phy_access": "mdio", |
| 66 | + "bus_id": 0, |
| 67 | + "context_id": 1, |
| 68 | + "macsec_ipg": 12, |
| 69 | + "macsec_supported": false, |
| 70 | + "hwinfo": "mdio0_0_0/0" |
| 71 | + }], |
| 72 | + "interfaces": [ |
| 73 | + { "name": "Ethernet0", "index": 1, "phy_id": 1, "system_lanes": [0,1,2,3], "line_lanes": [4,5,6,7] } |
| 74 | + ] |
| 75 | +} |
| 76 | +``` |
| 77 | + |
| 78 | +#### Behavior |
| 79 | +For each port: |
| 80 | +- If `macsec_supported == false`: use NPU/global switch for MACsec |
| 81 | +- If `macsec_supported == true` OR field is absent: use PHY switch for MACsec (default, preserves existing behavior) |
| 82 | + |
| 83 | +No runtime fallback. If PHY SAI does not implement MACsec and the field is true/absent, the operation fails and is logged. |
| 84 | + |
| 85 | +#### Component Changes |
| 86 | + |
| 87 | +### A) gearsyncd (JSON path only) |
| 88 | +Extend `GearboxParser` to accept optional `macsec_supported` in each `phys` item and write it into `_GEARBOX_TABLE` alongside other PHY attributes. |
| 89 | + |
| 90 | +File: `src/sonic-swss/gearsyncd/gearboxparser.cpp` |
| 91 | +Minimal addition inside the `phys` loop: |
| 92 | +```cpp |
| 93 | +if (phy.find("macsec_supported") != phy.end()) { |
| 94 | + val = phy["macsec_supported"]; |
| 95 | + attrs.emplace_back("macsec_supported", val.get<bool>() ? "true" : "false"); |
| 96 | +} |
| 97 | +``` |
| 98 | + |
| 99 | +### B) GearboxUtils (orchagent/lib) |
| 100 | +Extend `gearbox_phy_t` to include `macsec_supported` (default true if absent). Parse the field in `loadPhyMap()`. |
| 101 | + |
| 102 | +Files: `src/sonic-swss/lib/gearboxutils.h`, `src/sonic-swss/lib/gearboxutils.cpp` |
| 103 | +Minimal additions: |
| 104 | +```cpp |
| 105 | +// gearboxutils.h |
| 106 | +typedef struct { |
| 107 | + int phy_id; |
| 108 | + std::string phy_oid; |
| 109 | + std::string name; |
| 110 | + std::string lib_name; |
| 111 | + std::string firmware; |
| 112 | + std::string firmware_major_version; |
| 113 | + std::string sai_init_config_file; |
| 114 | + std::string config_file; |
| 115 | + std::string access; |
| 116 | + std::string hwinfo; |
| 117 | + uint32_t address; |
| 118 | + uint32_t bus_id; |
| 119 | + uint32_t context_id; |
| 120 | + uint32_t macsec_ipg; |
| 121 | + bool macsec_supported; // new, default true |
| 122 | +} gearbox_phy_t; |
| 123 | + |
| 124 | +// gearboxutils.cpp (in loadPhyMap value loop) |
| 125 | +bool cap = true; // default |
| 126 | +for (auto &fv : ovalues) { |
| 127 | + if (fv.first == "macsec_supported") cap = (fv.second == "true"); |
| 128 | +} |
| 129 | +phy.macsec_supported = cap; |
| 130 | +``` |
| 131 | + |
| 132 | +### C) MACsec orchestration (orchagent) |
| 133 | +Decide backend per port using the associated PHY capability from GearboxUtils: |
| 134 | +- If the mapped PHY has `macsec_supported == false` → use `gSwitchId` and NPU front-panel port OID |
| 135 | +- Otherwise (true or absent) → use PHY switch (default) |
| 136 | + |
| 137 | + |
| 138 | + |
| 139 | +File: `src/sonic-swss/orchagent/macsecorch.cpp` |
| 140 | +Decision skeleton: |
| 141 | +```cpp |
| 142 | +bool use_phy = true; // default preserves existing behavior |
| 143 | +if (phy && (phy->macsec_supported == false)) |
| 144 | +{ |
| 145 | + SWSS_LOG_NOTICE("MACsec: %s -> backend=NPU (phy marked unsupported)", port_name.c_str()); |
| 146 | + use_phy = false; |
| 147 | +} |
| 148 | +return use_phy ? usePhy() : useNpu(); |
| 149 | +``` |
| 150 | + |
| 151 | +#### Control Flow |
| 152 | +1) Boot: |
| 153 | + - gearsyncd consumes gearbox_config.json and writes `_GEARBOX_TABLE` with `macsec_supported` per PHY (if present). |
| 154 | +2) MACsec enable on port: |
| 155 | + - MACsecOrch reads the mapped PHY capability via GearboxUtils → chooses backend (PHY if supported/absent; NPU only if explicitly `false`). |
| 156 | + |
| 157 | + |
| 158 | +Control flow diagram: |
| 159 | +```mermaid |
| 160 | +flowchart LR |
| 161 | + A["Boot"] --> B["gearbox_config.json parsed by gearsyncd"] |
| 162 | + B --> C["_GEARBOX_TABLE: phy(id) macsec_supported"] |
| 163 | + C --> D["MACsec enable on port"] |
| 164 | + D --> E["Lookup mapped PHY via GearboxUtils"] |
| 165 | + E --> F{"PHY macsec_supported?"} |
| 166 | + F -- "false" --> G["Select NPU backend (gSwitchId)"] |
| 167 | + F -- "true or absent" --> H["Select PHY backend"] |
| 168 | + G --> I["Proceed with MACsec ops"] |
| 169 | + H --> I |
| 170 | +``` |
| 171 | + |
| 172 | +#### Error Handling |
| 173 | +- Missing `macsec_supported`: treat as supported; select PHY; NOTICE log if needed. |
| 174 | +- Missing `_GEARBOX_TABLE`: default to PHY (preserve existing behavior); NOTICE log. |
| 175 | +- SAI NOT_IMPLEMENTED on PHY: no fallback; operation fails; ERROR log. |
| 176 | + |
| 177 | +## Configuration and management |
| 178 | +- No CLI knobs added; platform owners annotate HWSKU gearbox_config.json (per-PHY) with optional macsec_supported |
| 179 | +- gearsyncd consumes gearbox_config.json; writes _GEARBOX_TABLE (phy:<id>) with macsec_supported |
| 180 | +- MACsecOrch selects backend per port using the mapped PHY capability; no runtime probing/fallback |
| 181 | + |
| 182 | +## Warmboot and Fastboot Design Impact |
| 183 | +- No change; backend selection is deterministic and re-derived on restart |
| 184 | + |
| 185 | +## Backward Compatibility |
| 186 | +- Field is optional; absent implies PHY behavior (preserves existing default) |
| 187 | +- No operator knobs added; no change to APP/CONFIG schema semantics |
| 188 | +- Rolling upgrades safe |
| 189 | + |
| 190 | +## Testing Requirements/Design |
| 191 | +- DVS integration tests: |
| 192 | + - Seed _GEARBOX_TABLE with and without macsec_supported; enable MACsec; verify backend selection and NOTICE logs |
| 193 | + - Verify MACsec objects created in ASIC_DB (NPU) when unsupported; and in GB_ASIC_DB only when PHY supports and SAI implements |
| 194 | +- Unit tests: |
| 195 | + - GearboxUtils: parse macsec_supported from APP_DB entries |
| 196 | +- Existing tests (tests/test_macsec.py) continue to pass |
0 commit comments