CLI cpo-util HLD by Junchao-Mellanox · Pull Request #2489 · sonic-net/SONiC

Junchao-Mellanox · 2026-08-07T09:57:29Z

No description provided.

mssonicbld · 2026-08-07T09:57:37Z

/azp run

azure-pipelines · 2026-08-07T09:57:43Z

No pipelines are associated with this pull request.

mssonicbld · 2026-08-07T10:21:32Z

/azp run

azure-pipelines · 2026-08-07T10:21:38Z

No pipelines are associated with this pull request.

mssonicbld · 2026-08-12T10:14:41Z

/azp run

azure-pipelines · 2026-08-12T10:14:48Z

No pipelines are associated with this pull request.

prgeor · 2026-08-12T17:01:57Z

+| commit | Set the new firmware as the default. | Yes |
+| upgrade | Perform download, run, and commit. | Yes |
+| unlock | Enter module password. | No |
+| target | Select target firmware for `Y-cable`. | No (not relevant) |


@Junchao-Mellanox Need an option to be able to select the target so that all subsequent CDB commands is directed to the targeted end point (Eg, MCU, OE, ELSP) on that port#

We can choose it from --component option, which supports: oe | els | all

prgeor · 2026-08-12T17:03:59Z

+
+Firmware execution is delegated only after `cpoutil` completes parsing, matching, and optional checksum verification. Failures are isolated per component; a failed ELS does not roll back a successful OE. The CLI reports status per internal component identity. Fatal conditions yield a non-zero exit; per-target warnings do not mask fatal errors.
+
+**Frontend (CLI):**


@Junchao-Mellanox Please add the CLI output. User should be able to see the progress of the operation (download) and if there is a error from CDB transaction, the CLI should capture the Error code mentioned in the CDB/CMIS spec

Will add it later.

prgeor · 2026-08-12T17:14:12Z

+
+Corresponding CLI documentation should be added to https://github.com/sonic-net/sonic-utilities/blob/master/doc/Command-Reference.md when the feature is implemented.
+
+> Note: Only one CLI instance may run at a time. This constraint should be enforced with a file lock.


@Junchao-Mellanox for simplicity we can leave this to the user where multiple ports can share resources MCU/OE/ELSP. This is same as we do for 8x100G on pluggable modules.

prgeor · 2026-08-12T17:21:17Z

+> Note: currently, ElsfpApi is not inherit from CmisApi, each vendor has to implement `get_fw_ops_match_attrs` by their own.
+
+
+#### 7.7 Error handling


@Junchao-Mellanox @eddyk-nvidia I would wish if this HLD can be split into two phases

Basic CPO component firmware upgrade via CLI , download, run, commit using port#, firmware binary file

OIF based package which is built on top of app table schema #1

This gives the user flexibility to build their own services around #1 (who don't want to pursue OIF package based approach). Its also easier to debug issues using #1 and much simpler to implement and on par with pluggable firmware download procedure.

bgallagher-nexthop · 2026-08-13T16:53:57Z

+Matching stage outputs a list of `plan_entries`:
+
+```
+plan_entry {


Would it make sense to rename this match_entry and have the match_entries list only contain entries for matches? I am not sure of the use-case of a plan_entry that has matching_decision == None.

matching_decision = None is used for logging purposes. For example, if a user wants to upgrade Ethernet0 and Ethernet120, but only Ethernet0 has a matching entry, we need to log that Ethernet120 has no match. Maybe we don't really need this entry, we will see in the implementation and update acoordingly.

bgallagher-nexthop · 2026-08-13T18:55:52Z

+- Add a new function `get_fw_ops_match_attrs` to get all the FW operation matching attributes on demand.
+- `ElsfpApi` should be updated to inherit from, or mix in, `CmisCdbFw` so that it can support CDB-based firmware operations. (Or, should each vendor provide a `VendorCdbFw`?)
+
+> Note: currently, ElsfpApi is not inherit from CmisApi, each vendor has to implement `get_fw_ops_match_attrs` by their own.


If get_fw_ops_match_attrs was moved into the CmisCdbFw mixin, would this be solved? CmisApi and ElsfpApi would then share the same definition via the mixin.

Moving it to CmisCdbFw is an option. Another option is to put it to XcvrApi. I will compare the 2 options and update.

bgallagher-nexthop · 2026-08-13T19:13:03Z

+from dataclasses import dataclass
+
+@dataclass(slots=True)
+class CpoFwOpsEntry:


We could keep these cpoutil related data structures out of the platform API by instead adding the download, run, commit and upgrade methods on CpoDeviceBase instead.

Then both the OeBase and ElsfpBase classes will expose the firmware methods, but we no longer need the CpoFwOpsEntry to communicate the component information. Instead, cpoutil will just access the device it cares about based on the matching firmware operation entry it is about to perform an upgrade for:

# in cpoutil cpo = get_cpo_object(fw_op_entry) if fw_op_entry.component_class == "OpticalEngine": cpo.oe.upgrade_firmware(file_path=fw_op_entry.file_path) elif fw_op_entry.component_class == "ExternalLaserSource": cpo.elsfp.upgrade_firmware(file_path=fw_op_entry.file_path)

I prefer the current way because: implementing it in CpoBase will give the max flexibility for vendor to support different hardware topology. For example, it support both joint mode and separate mode well. Implementing it in OeBase and ElsfpBase will limit it in separate mode.

I am not sure I understand, both approaches should support arbitrary hardware topologies (separate/joint). There will always be OE and ELSFP objects present on the CpoBase object, regardless of whether the hardware operates in joint or separate mode.

The only change in my suggestion is that we don't leak cpoutil's abstraction into the platform API by removing the need to add CpoFwOpsEntry to the platform API entirely. Instead, we just add the method definitions and let cpoutil do the decision making of selecting the appropriate component to operate on instead of the platform API doing that. The main motivations being:

cpoutil's data model (CpoFwOpsEntry) ideally should not be added to the platform API as it tightly couples cpoutil and the platform API

By keeping CpoFwOpsEntry in cpoutil it makes any changes to the data model easier since you don't have to do cross-repo changes. Imagine a new field had to be added to CpoFwOpsEntry, then you might need two PRs across two repos -- one to add the field, one to plumb it through in cpoutil.

Defining methods on CpoBase can be useful for backwards compatibility, so you can treat the OE/ELSFP as a single aggregated device like some form of pluggable transceiver. Since cpoutil is new code, this doesn't apply and we are in the convenient position to just reason about the OE and ELSFP directly.

mssonicbld · 2026-08-14T01:41:27Z

/azp run

azure-pipelines · 2026-08-14T01:41:36Z

No pipelines are associated with this pull request.

Junchao-Mellanox force-pushed the master-cpo-util branch from 9720325 to 8d14bec Compare August 7, 2026 10:21

yoninabedrik mentioned this pull request Aug 10, 2026

CPO Firmware Upgrade #2487

Open

CLI cpo-util HLD

a049b79

Junchao-Mellanox force-pushed the master-cpo-util branch from 8d14bec to a049b79 Compare August 12, 2026 10:14

prgeor reviewed Aug 12, 2026

View reviewed changes

bgallagher-nexthop reviewed Aug 13, 2026

View reviewed changes

Fix review comments

415630b


		Firmware execution is delegated only after `cpoutil` completes parsing, matching, and optional checksum verification. Failures are isolated per component; a failed ELS does not roll back a successful OE. The CLI reports status per internal component identity. Fatal conditions yield a non-zero exit; per-target warnings do not mask fatal errors.

		Frontend (CLI):


		Corresponding CLI documentation should be added to https://github.com/sonic-net/sonic-utilities/blob/master/doc/Command-Reference.md when the feature is implemented.

		> Note: Only one CLI instance may run at a time. This constraint should be enforced with a file lock.

		> Note: currently, ElsfpApi is not inherit from CmisApi, each vendor has to implement `get_fw_ops_match_attrs` by their own.


		#### 7.7 Error handling

Uh oh!

Conversation

Junchao-Mellanox commented Aug 7, 2026

Uh oh!

mssonicbld commented Aug 7, 2026

Uh oh!

azure-pipelines Bot commented Aug 7, 2026

Uh oh!

mssonicbld commented Aug 7, 2026

Uh oh!

azure-pipelines Bot commented Aug 7, 2026

Uh oh!

mssonicbld commented Aug 12, 2026

Uh oh!

azure-pipelines Bot commented Aug 12, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Junchao-Mellanox Aug 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Junchao-Mellanox Aug 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mssonicbld commented Aug 14, 2026

Uh oh!

azure-pipelines Bot commented Aug 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Junchao-Mellanox Aug 14, 2026 •

edited

Loading

Junchao-Mellanox Aug 14, 2026 •

edited

Loading