[xcvr]: Fix appl_advt dict-lookup for non-sequential AppSel indices

[gs1571]

Description

get_host_lane_count(), get_media_lane_count() and get_media_lane_assignment_option()
in cmis.py guard their dict lookup into the application advertisement with
len(appl_advt) >= appl. That holds only when the advertised Applications form a
contiguous 1..N range, and returns 0 for a valid Application on a sparse
advertisement, where appl in appl_advt is true but len(appl_advt) < appl.

Changed the guard to appl in appl_advt, matching the pattern already used by
get_host_lane_assignment_option() in the same file.

Motivation and Context

get_application_advertisement() is best-effort: it skips any Application whose
host/media interface code cannot be decoded (an unknown SFF-8024 code decodes to
Unknown and is dropped). The advertisement dict is therefore allowed to be
sparse / non-contiguous, so consumers must look Applications up by key rather
than assume the keys are 1..N. Under the old guard, when a module advertises an
Application at an index higher than the number of decoded entries (e.g. appl=9
with len(appl_advt) == 4), these getters return 0 for a valid Application.

How Has This Been Tested?

Added regression cases with a sparse appl_advt (single high-index entry) for all
three affected functions in tests/sonic_xcvr/test_cmis.py, plus a case confirming
a genuinely-absent index still returns 0.

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: gs1571 / name: Grigory Solovyev (15356cb)

[mssonicbld]

/azp run

[azure-pipelines]

Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[mssonicbld]

Hi, there are workflow run(s) waiting for approval, you may be first-time contributor. I will notify maintainers to help approve once PR is approved. Thanks!

_{---Powered by SONiC BuildBot}

[bgallagher-nexthop]

Thinking further: a module that advertises only a single application, and it is not the first application descriptor (app sel code 1) is not compliant with the CMIS spec:

"A module advertises at least one supported Application as its power-up default Application in the first Application Descriptor (identified by AppSel code 1), and it advertises any additionally supported types of Applications consecutively..."

Looking at the code for get_application_advertisements, it seems it is possible that we would skip applications we have no mapping for because decoding can pass back "Unknown" for missing entries and the get_application_advertisements code seems like it could skip those cases and pass back a dictionary with gaps in the app sel codes.

I don't think it seems likely that the current fix addresses the root cause as result.

It seems like either one of the two is the root cause:

• we are missing entries in our sff-8024 codes for host/media interfaces
• the module firmware is potentially buggy

[bgallagher-nexthop]

(Marking request changes to dismiss my approval based on above comment)

[aditya-nexthop]

Found while bringing up an 800G ZR+ coherent module (CMIS 5.3) whose only
advertised Lower-Memory Application is at a non-1 index

Please can you explain how a module does this? If the only advertised Lower-Memory Application is at a non-1 index, what's in the application advertisements prior (1 to n) to this application?

It would help if you can post either an EEPROM dump of the advertising region or a list of applications this module advertises.

[gs1571]

@bgallagher-nexthop @aditya-nexthop

Went back and tried to reproduce this on the actual module. What I could reproduce is a different, and I think more likely, cause — the one @bgallagher-nexthop suggested: our SFF-8024 table not covering all of the module's MediaIF codes.

Setup: an 800G ZR+ coherent module (CMIS 5.3) that, with full SFF-8024 coverage, advertises a clean, contiguous appl_advt for apps 1-12, no gaps.

I removed 2 of the 3 SFF-8024 entries this module needs (kept the third), then restarted xcvrd. Result:

appl_advt = {
    9:  {'module_media_interface_id': '<code>', 'host_lane_count': 8, ...},
    10: {...},
    11: {...},
    12: {...},
}

Apps 1-8 disappeared — get_application_advertisements() decoded their MediaIF codes to Unknown and skipped them, exactly as you described. Apps 9-12 stayed because their code was still in the table.

xcvrd acted on this dict directly:

Setting appl=9
Setting host_lanemask=0xff
Setting media_lanemask=0x1

len(appl_advt) == 4 here. Under the old guard, 4 >= 9 is False — same for 10, 11, 12. So get_media_lane_count() / get_host_lane_assignment_option() would return 0 for all four Applications xcvrd was actually using.

So: I can't prove the module itself is non-compliant, and I'm dropping that claim. But an incomplete SFF-8024 table — which is a moving target, given how often new codes get added (#601, #687, #701 alone span about 9 months) — produces the exact same non-contiguous appl_advt shape, and the current guard breaks on it the same way. I'll update the PR description to reflect this instead of the original claim. Let me know if you still want changes to the fix itself, or if the corrected motivation resolves the concern.

[aditya-nexthop]

LGTM

[aditya-nexthop]

please don't leave comments about code that no longer exists. It only confuses readers.

[aditya-nexthop]

please don't leave comments about code that no longer exists. It only confuses readers.

[aditya-nexthop]

same

[bgallagher-nexthop]

The fix makes sense to me when framed as striving for best-effort application advertisement parsing in the presence of an incomplete/outdated SFF-8024 identifier set (software allows a sparse app advertisement dict since it skips applications it could not successfully decode). Please also update the PR description to reflect this reasoning for the fix.

[mssonicbld]

/azp run

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[gs1571]

Thanks for your comments. Addressed both asks from this round: reworded the three sparse-appl_advt test comments to drop references to the removed len(appl_advt) >= appl guard, and updated the PR description to frame the fix as best-effort parsing of a sparse application advertisement. The fix itself (appl in appl_advt in the three getters) is unchanged; all test_cmis.py tests pass.

No changes to the fix itself (appl in appl_advt in the three getters). Let me know if anything else is needed.