search

sonic-net / sonic-platform-daemons

Platform module daemons for SONiC
Other
23 stars 152 forks source link

[xcvrd] DPB support on platforms with CmisManagerTask enabled #500

Open ishidawataru opened 4 months ago

ishidawataru commented 4 months ago

Description

This commit fixes DPB support with CMIS transceivers.

CmisManagerTask's port_dict must be updated according to the port add/remove events. This commit removes the port_mapping field from CmisManagerTask as port_mapping was mostly used just for storing asic_id information and that can be simply done by port_dict instead.

Added a helper method get_asic_id() method to CmisManagerTask for getting asic_id from logical_port.

Motivation and Context

fixes https://github.com/sonic-net/sonic-buildimage/issues/18893

How Has This Been Tested?

I tested on the VS environment.

Additional Information (Optional)

linux-foundation-easycla[bot] commented 4 months ago

CLA Signed

The committers listed above are authorized under a signed CLA.

mihirpat1 commented 4 months ago

@ishidawataru Can you please fix the code coverage check failure?

ishidawataru commented 4 months ago

@mihirpat1 I'm still communicating with the original reporter of this issue to check the fix actually works. After that I'll fix the code coverage failure.

ishidawataru commented 3 months ago

@mihirpat1 Confirmed that the fix resolved the issue. Also, I fixed the coverage test failure. Please review.

ishidawataru commented 3 months ago

@prgeor This PR fixes https://github.com/sonic-net/sonic-buildimage/issues/18893. It is not about the application selection. The current CmisManagerTask implementation doesn't delete the port info from port_dict when ports are deleted due to DPB configuration, which causes a crash.

ishidawataru commented 1 month ago

@longhuan-cisco @mihirpat1 I updated the PR based on @longhuan-cisco 's suggestion. Please review.

I used force-push and removed the original commits. I backed up the original branch here.