Skip to content

Commit 7e40f37

Browse files
NO-ISS: MACsec gearbox: Deterministic MACsec backend selection for gearbox ports
1 parent 4fc4f48 commit 7e40f37

File tree

1 file changed

+208
-0
lines changed

1 file changed

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

0 commit comments

Comments
 (0)