[xcvrd] Add Initial CPO Support - #843
Conversation
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
- Refactor CmisManagerTask to accept a dictionary of SFP objects - Add CPO DomInfoUpdateTask and CmisManagerTask skeletons that will handle any CPO ports Signed-off-by: Brian Gallagher <bgallagher@nexthop.ai>
960cebb to
4e904ca
Compare
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
Signed-off-by: Brian Gallagher <bgallagher@nexthop.ai>
Signed-off-by: Brian Gallagher <bgallagher@nexthop.ai>
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
Signed-off-by: Brian Gallagher <bgallagher@nexthop.ai>
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
Signed-off-by: Brian Gallagher <bgallagher@nexthop.ai>
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
Signed-off-by: Brian Gallagher <bgallagher@nexthop.ai>
Signed-off-by: Brian Gallagher <bgallagher@nexthop.ai>
|
/azp run |
| class CpoManagerTask(CmisManagerTask): | ||
| def __init__(self, namespaces, port_mapping, port_obj_dict, main_thread_stop_event, skip_cpo_mgr=False): | ||
| super().__init__(namespaces, port_mapping, port_obj_dict, main_thread_stop_event, | ||
| skip_cmis_mgr=skip_cpo_mgr) |
There was a problem hiding this comment.
@bgallagher-nexthop I don't see any use case for skip . can we remove? Cpo manager is mandatory for CPO platforms. Also, the skip cmis_mgr is not applicable for CPO platforms. A cpo platform can have two independent task CpoManagerTask and CmisManagerTask but when you pass the skip_cmis_mgr inside CpoManagerTask, its confusing.
Lets create the task based upon port dict has sfp or cpo objects
There was a problem hiding this comment.
This was requested by @tshalvi here, for platforms that may manage the state machine via firmware: sonic-net/SONiC#2444 (comment)
Do you require this functionality @tshalvi ?
Signed-off-by: Brian Gallagher <bgallagher@nexthop.ai>
e8509e9
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
Signed-off-by: Brian Gallagher <bgallagher@nexthop.ai>
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
There was a problem hiding this comment.
Pull request overview
This PR refactors sonic-xcvrd to begin supporting Co-Packaged Optics (CPO) ports alongside traditional pluggable transceivers, by splitting port object management and introducing initial CPO task scaffolding so existing CMIS/DOM tasks don’t conflict with future CPO-specific logic.
Changes:
- Introduces initial CPO task classes (manager/state/DOM update) and wires them into
xcvrdstartup/shutdown sequencing. - Adds shared utilities for resolving a “port device” (CPO vs pluggable) and building per-port device dictionaries.
- Refactors CMIS and DOM tasks to operate only on ports provided via
port_obj_dict, and updates unit tests accordingly.
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| sonic-xcvrd/xcvrd/xcvrd.py | Wires in CPO task threads; builds separate pluggable vs CPO device dicts. |
| sonic-xcvrd/xcvrd/xcvrd_utilities/common.py | Adds CPO/pluggable device resolution helpers and dict builders. |
| sonic-xcvrd/xcvrd/dom/dom_mgr.py | Refactors DOM tasks to use port_obj_dict and skips ports not in-scope for the task. |
| sonic-xcvrd/xcvrd/cpo/dom_mgr.py | Adds CpoDomInfoUpdateTask scaffold inheriting DOM updater behavior. |
| sonic-xcvrd/xcvrd/cpo/cpo_state_task.py | Adds CpoStateUpdateTask scaffold inheriting state updater behavior. |
| sonic-xcvrd/xcvrd/cpo/cpo_manager_task.py | Adds CpoManagerTask scaffold inheriting CMIS manager behavior. |
| sonic-xcvrd/xcvrd/cpo/init.py | Initializes the new cpo package. |
| sonic-xcvrd/xcvrd/cmis/cmis_manager_task.py | Refactors CMIS manager to use passed-in port objects and ignore out-of-scope ports. |
| sonic-xcvrd/tests/test_xcvrd.py | Updates tests for the new task signatures and adds coverage for mixed CPO+pluggable task creation. |
| sonic-xcvrd/tests/test_cpo.py | Adds focused unit tests for new CPO/pluggable resolver utilities. |
Suppressed comments (1)
sonic-xcvrd/tests/test_xcvrd.py:5228
PortMappingnormalizesport_indextointandget_physical_to_logical()asserts the physical port is anint. Using string keys here ('1') diverges from production behavior and can mask issues with the newphysical_port not in port_obj_dictfiltering. Use integer physical-port keys consistently.
port_mapping = PortMapping()
mock_sfp_obj_dict = {'1': MagicMock()}
stop_event = threading.Event()
mock_cmis_manager = MagicMock()
task = DomInfoUpdateTask(DEFAULT_NAMESPACE, port_mapping, mock_sfp_obj_dict, stop_event, mock_cmis_manager, 0)
task.xcvr_table_helper = XcvrTableHelper(DEFAULT_NAMESPACE)
task.task_stopping_event.is_set = MagicMock(side_effect=[False, False, False, True])
task.port_mapping.logical_port_list = ['Ethernet0']
task.port_mapping.physical_to_logical = {'1': ['Ethernet0']}
Signed-off-by: Brian Gallagher <bgallagher@nexthop.ai>
|
/azp run |
|
Azure Pipelines will not run the associated pipelines, because the pull request was updated after the run command was issued. Review the pull request again and issue a new run command. |
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
Signed-off-by: Brian Gallagher <bgallagher@nexthop.ai>
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
Description
This PR performs some basic refactoring required to fully implement the changes proposed in the CPO support in xcvrd HLD.
Initially, it does the following:
CpoManagerTask,CpoDomInfoUpdateTaskandCpoStateUpdateTask). These classes will be extended in later PRs to actually contain any necessary logic specific to CPO.sfp_obj_dicttoport_obj_dictin most task code, since these dictionaries can now contain eitherSfpBaseorCpoBaseobjects.CmisManagerTaskandDomInfoUpdateTaskto only operate against the ports that are supplied to the task viaport_obj_dict. This will prevent the existing tasks from conflicting with the CPO tasks.is_cpo_port,is_pluggable_port,get_port_device, etc).Motivation and Context
These changes are required to add support for CPO hardware to
xcvrd. See the CPO support in xcvrd HLD for more detail.How Has This Been Tested?
Added unit-tests.
Tested manually on a non-CPO switch to ensure no regressions for existing hardware platforms.
xcvrd.