cmisVDM: cache the static VDM descriptor pages

sonic_platform_base/sonic_xcvr/api/public/cmisVDM.py

@@ -31,7 +31,33 @@ class CmisVdmApi(XcvrApi):
 
     def __init__(self, xcvr_eeprom):
         super(CmisVdmApi, self).__init__(xcvr_eeprom)
-
+        # Raw VDM descriptor pages, keyed by page. Populated lazily by
+        # _read_vdm_descriptor_page; recreated with the api object on OIR/reboot.
+        self._vdm_descriptor = {}
+
+    def _read_vdm_descriptor_page(self, page):
+        '''
+        Read a raw VDM descriptor page (0x20-0x23), caching it on the instance.
+
+        The VDM descriptors -- observable Type ID, threshold-set ID, and
+        monitored-lane assignment for each of the 64 slots in the page -- are a
+        static, read-only module advertisement in CMIS: they describe how the
+        VDM value/threshold pages are laid out and do not change while the
+        module is powered/plugged in. So the descriptor page is read once and
+        reused across DOM cycles, removing one 128 B page read per descriptor
+        page per port per cycle.
+
+        Only a non-empty read is cached, so a transient read failure is retried
+        on the next cycle rather than disabling VDM for the life of the api
+        object. Pages are cached independently, and only the pages actually
+        requested are read. The cache lives on the api object, which xcvrd
+        recreates on module re-insertion, so it needs no explicit invalidation.
+        '''
+        if not self._vdm_descriptor.get(page):
+            offset = page * PAGE_SIZE + PAGE_OFFSET
+            self._vdm_descriptor[page] = self.xcvr_eeprom.read_raw(offset, PAGE_SIZE)
+        return self._vdm_descriptor[page]
+
     def get_F16(self, value):
         '''
         This function converts raw data to "F16" format defined in cmis.
@@ -69,7 +95,7 @@ def get_vdm_page(self, page, VDM_flag_page, field_option=ALL_FIELD, observable_t
         '''
         if page not in [0x20, 0x21, 0x22, 0x23]:
             raise ValueError('Page not in VDM Descriptor range!')
-        vdm_descriptor = self.xcvr_eeprom.read_raw(page * PAGE_SIZE + PAGE_OFFSET, PAGE_SIZE)
+        vdm_descriptor = self._read_vdm_descriptor_page(page)
         if not vdm_descriptor:
             return {}
 

tests/sonic_xcvr/test_cmisVDM.py

@@ -1,17 +1,21 @@
 from mock import MagicMock
 import pytest
-from sonic_platform_base.sonic_xcvr.api.public.cmisVDM import CmisVdmApi
+from collections import Counter
+from sonic_platform_base.sonic_xcvr.api.public.cmisVDM import CmisVdmApi, PAGE_SIZE, PAGE_OFFSET
 from sonic_platform_base.sonic_xcvr.mem_maps.public.cmis import CmisMemMap
 from sonic_platform_base.sonic_xcvr.xcvr_eeprom import XcvrEeprom
 from sonic_platform_base.sonic_xcvr.codes.public.cmis import CmisCodes
 
 class TestVDM(object):
     codes = CmisCodes
     mem_map = CmisMemMap(codes)
-    reader = MagicMock(return_value=None)
-    writer = MagicMock()
-    eeprom = XcvrEeprom(reader, writer, mem_map)
-    api = CmisVdmApi(eeprom)
+
+    def setup_method(self):
+        # Fresh api per test: the descriptor-page cache lives on the api
+        # instance, so a shared api would serve one test's cached descriptor to
+        # the next instead of the descriptor that test mocks.
+        eeprom = XcvrEeprom(MagicMock(return_value=None), MagicMock(), self.mem_map)
+        self.api = CmisVdmApi(eeprom)
 
     @pytest.mark.parametrize("input_param, expected", [
         (0x9200, 0.000512)
@@ -97,6 +101,68 @@ def test_get_vdm_page_none_vdm_descriptor(self, input_param, mock_response, expe
         result = self.api.get_vdm_page(*input_param)
         assert result == expected
 
+    # --- VDM descriptor-page caching (descriptors are static per CMIS) ---
+
+    def _make_vdm_api_with_counting_reader(self):
+        """Fresh CmisVdmApi whose read_raw returns by page and counts reads per
+        page offset, so a cache test can assert the descriptor page is read once
+        across cycles while the value/threshold pages are re-read every cycle."""
+        eeprom = XcvrEeprom(MagicMock(), MagicMock(), CmisMemMap(CmisCodes))
+        api = CmisVdmApi(eeprom)
+        reads = Counter()
+        descriptor = tuple([16, 9] + [0] * (PAGE_SIZE - 2))  # one valid observable
+
+        def read_raw(offset, size, *args, **kwargs):
+            reads[offset] += 1
+            page = (offset - PAGE_OFFSET) // PAGE_SIZE
+            if page in (0x20, 0x21, 0x22, 0x23):
+                return descriptor[:size]
+            # value / threshold pages: return exactly the requested width, since
+            # get_vdm_page struct-unpacks these reads (2 B value, 8 B threshold)
+            return bytearray(size)
+
+        api.xcvr_eeprom.read_raw = read_raw
+        return api, reads
+
+    def test_vdm_descriptor_page_cached(self):
+        api, reads = self._make_vdm_api_with_counting_reader()
+        desc_off = 0x20 * PAGE_SIZE + PAGE_OFFSET
+        val_off = 0x24 * PAGE_SIZE + PAGE_OFFSET
+        for _ in range(3):
+            api.get_vdm_page(0x20, None)
+        # descriptor read once and reused; value page re-read every cycle
+        assert reads[desc_off] == 1
+        assert reads[val_off] == 3
+
+    def test_vdm_descriptor_cache_is_per_page(self):
+        api, reads = self._make_vdm_api_with_counting_reader()
+        for page in (0x20, 0x21, 0x20, 0x21):
+            api.get_vdm_page(page, None)
+        assert reads[0x20 * PAGE_SIZE + PAGE_OFFSET] == 1
+        assert reads[0x21 * PAGE_SIZE + PAGE_OFFSET] == 1
+
+    def test_vdm_descriptor_empty_read_not_cached(self):
+        """A transient empty descriptor read must not be cached -> retried."""
+        eeprom = XcvrEeprom(MagicMock(), MagicMock(), CmisMemMap(CmisCodes))
+        api = CmisVdmApi(eeprom)
+        descriptor = tuple([16, 9] + [0] * (PAGE_SIZE - 2))
+        seq = [None, descriptor, descriptor]  # first read fails, then succeeds
+        calls = {"n": 0}
+
+        def read_raw(offset, size, *args, **kwargs):
+            page = (offset - PAGE_OFFSET) // PAGE_SIZE
+            if page == 0x20:
+                v = seq[calls["n"]] if calls["n"] < len(seq) else descriptor
+                calls["n"] += 1
+                return v[:size] if v else v
+            return bytearray(size)
+
+        api.xcvr_eeprom.read_raw = read_raw
+        assert api.get_vdm_page(0x20, None) == {}      # empty descriptor -> {}
+        api.get_vdm_page(0x20, None)                   # retried, now populates cache
+        api.get_vdm_page(0x20, None)                   # served from cache
+        assert calls["n"] == 2                          # two descriptor reads, not three
+
     def test_get_vdm_page_observable_type_basic_only(self):
         """Test get_vdm_page with VDM_OBSERVABLE_BASIC filters out statistic observables"""
         # Descriptor: typeIDs at odd positions: 9(S), 11(S), 13(S), 15(B), 10(S), 10(S), 0, 0