Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for port_id in object discovery for dpu #171

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add support for port_id in object discovery for dpu #171

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 0 additions & 11 deletions common/sai.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -489,17 +489,6 @@ def objects_discovery(self):
for idx, oid in enumerate(port_oids):
self.create_alias(f"PORT_{idx}", 'SAI_OBJECT_TYPE_PORT', oid)


status, data = self.get(dot1q_br_oid, ["SAI_BRIDGE_ATTR_PORT_LIST", "1:oid:0x0"], False)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this was removed?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is removed since its not yet implemented for bmv2. This is the reason for the lines to be deleted. These were causing issues for PORT_ID object discovery. Once it is implemented in bmv2 The lines can be put back in place.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BMv2 is not the only target for SAI-C. It works for many other ASICs. Removing the code is not the right way to fix the issues. As an option, in case BMv2 returns SAI_STATUS_NOT_IMPLEMENTED or SAI_STATUS_NOT_SUPPORTED, some additional check can be added instead.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what will be a good way to make it work on BMv2 as well considering BMv2 does not support those SAI calls, should we add a if dpu dont execute the code else if npu execute it ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mgheorghe , in case BMv2 SAI implementation does not support these calls, it should return SAI_STATUS_NOT_SUPPORTED or SAI_STATUS_NOT_IMPLEMENTED status code and we should add appropriate status check here. In case for some reason it returns SAI_STATUS_SUCCESS or just crashes (both options I'd consider as DASH/BMv2 bug), we can add a workaround-check on ASIC type which is initialized here:
https://github.com/opencomputeproject/SAI-Challenger/blob/40e09a6c17bad1bc35c57772cdf6bbebd3387a50/common/sai_dpu.py#L10C1-L10C1

bport_num = data.uint32()
assert (status == "SAI_STATUS_BUFFER_OVERFLOW")
assert (bport_num > 0)

dot1q_bp_oids = self.get(dot1q_br_oid,
["SAI_BRIDGE_ATTR_PORT_LIST", self.make_list(bport_num, "oid:0x0")]).oids()
for idx, oid in enumerate(dot1q_bp_oids):
self.create_alias(f"BRIDGE_PORT_{idx}", 'SAI_OBJECT_TYPE_BRIDGE_PORT', oid)

def __apply_rec(self, fname):
# Since it's expected that sairedis.rec file contains a full configuration,
# we must flush both Redis and NPU state before we start.
Expand Down