facebook FBOSS Code Standards
Overview
Passive coding guidance for FBOSS. Applied automatically while writing or modifying code under fboss/.
Scope
Currently fboss/ only. TODO: extend to configerator/source/neteng/fboss, neteng/netcastle, neteng/fboss.
Quick Checklist
| Area | Pattern | Check |
|---|---|---|
| Agent | Mono/multi-switch duality | State changes must work in both modes |
| Agent | SW/HW build boundary | Production HW-agent executables do not depend on SW-agent implementation libraries |
| SAI/Build | Executable symbol exports | Export only needed SDK symbols in opt; keep sanitizer exports working |
| Agent | Warmboot serialization | New SwitchState fields must serialize/deserialize |
| SAI | SaiApiTable registration | New SAI attributes must be registered |
| SAI | SaiStore consistency | SAI objects must be tracked, no orphans |
| FSDB | State/Stats duality | Always handle both trees |
| FSDB | extern template | New ThriftStructNode instantiations need extern template |
| thrift_cow | COW modification | Use modify(), never mutate shared nodes |
| Platform | JSON + Thrift sync | Config changes update both schemas |
| Testing | Naming convention | Follow AgentHw<Feature>Test pattern |
| Testing | NSDB impact | Core FSDB changes must run NSDB tests |
| General | Follow local patterns | Be consistent with existing code in the directory |
| Testing | No GTEST_SKIP | Use ProductionFeatures filtering, never GTEST_SKIP() |
| Testing | DSF counters | Check reassembly errors on fabric ports, not discards |
| Agent | Non-coalescing behavioral deltas | Mark delta-significant updates non-coalescing |
| Agent | Rolled-out flag cleanup | Remove feature flags that are fully rolled out |
| Agent | Early return on empty deltas | Check for empty deltas and return early |
| Agent | No hardcoded ASIC types | Use feature/property lookups, not ASIC name checks |
| SAI | Explicit cancellation | Distinguish reconnectable errors from shutdown signals |
| General | No private fn defaults | Don't default parameters all callers override |
| General | Explicit state flags | Pass all enable/disable flags, no direction assumptions |
| General | Verify before deleting | Provide evidence files are unused before deleting |
| General | CHECK over assert | Use CHECK for production invariants, not assert() |
| General | emplace over operator[] | Use emplace/insert for map insertions |
| General | ASIC feature gating | Gate HW-specific constraints with ASIC features |
| General | Concise method naming | Don't include class name in method name |
| General | Explicit operator precedence | Parenthesize compound boolean/comparison expressions |
| General | Inline trivial comparisons | Don't wrap one-line checks in helper methods |
| General | Fix lint before landing | Run arc f and arc lint before submitting |
| Testing | Targeted unit tests | New modules need dedicated unit tests, not just HW tests |
| Agent | Documentation-value methods | Keep per-ASIC limit methods as architecture docs |
| General | Rate-limit verbose logging | Use XLOG_EVERY_MS, never log at line rate |
| General | Perspective-aware naming | Use fromX/toX instead of ambiguous Rx/Tx |
| General | Update OSS build files | Update CMake alongside BUCK when adding/moving files |
| Testing | Deep comparison over size checks | Compare content, not just collection size |
| Testing | Verify negative cases | Explicitly check absence, not just no-throw |
| Testing | Test overflow and error paths | Cover resource exhaustion, not just happy path |
| Testing | Optimize test execution time | Use smallest packet sizes, avoid unnecessary waits |
| Agent | ODS counters for debugging | Add ODS counters for drop/error events, not just logs |
| Agent | Validate external config limits | Reject unreasonable externally configurable values |
| Agent | Populate warmboot cache first | Fill all caches from warmboot state before processing |
| General | Empty vector over optional<vector> | Use empty vector, not optional<vector> |
| General | Map-based config | Use map lookups, not if-else chains for platform values |
| General | Log IDs + names | Include both numeric ID and name in log messages |
| Agent | Delta processing order | Remove → Change → Add; document in comments |
| Agent | Validator extraction | Extract complex validation into dedicated classes |
| Agent | Gate validations with flags | New enforcement behind FLAGS for gradual rollout |
| Testing | Test transient states | Test intermediate invalid states, not just final |
| Testing | Consolidate test helpers | Move shared helpers to base classes |
| Testing | Validation UTs | Pure validation → unit tests, not just HW tests |
| Testing | No setup changes to existing tests | Create new tests instead; setup changes break warmboot roundtrip CI |
| Testing | Program routes via routeUpdater | Use routeUpdater.program() (SwSwitchRouteUpdateWrapper); never hand-build routes and add to FIB; keep enable_nexthop_id_manager on so every route gets a nexthop ID |
| Agent | Read nexthops via ID-aware helpers | State/FIB-side code → FibHelpers getNextHops(state,…) etc.; RIB-internal code → *FromRib(manager,…); never getNextHopSet() (inline nexthops are being removed) |
When to Load References
| Topic | Action |
|---|---|
| Team coding conventions (C++ style, control flow, naming) | Read references/general-patterns.md |
| Agent code (SwSwitch, HwSwitch, mono/multi-switch, warmboot) | Read references/agent-patterns.md |
| SAI/SDK layer (SaiApi, SaiStore, SaiManager, vendor SDK) | Read references/sai-sdk-patterns.md |
| FSDB or thrift_cow (subscriptions, COW nodes, PatchBuilder) | Read references/thrift-cow-fsdb-patterns.md |
| Platform services or config (sensor_service, fan_service, JSON configs) | Read references/platform-config-patterns.md |
| Tests (agent HW tests, multinode tests, naming, fixtures) | Read references/testing-patterns.md |
| Adding new patterns | Read references/contributing.md |