From 0c6be3e520834575b867223e5648199fcd27e64e Mon Sep 17 00:00:00 2001
From: Aditya Bhiday <aditya@nexthop.ai>
Date: Tue, 30 Jun 2026 14:31:23 -0700
Subject: [PATCH 1/4] cmisVDM: cache the static VDM descriptor pages

Signed-off-by: Aditya Bhiday <aditya@nexthop.ai>
---
 .../sonic_xcvr/api/public/cmis.py             |  2 +
 .../sonic_xcvr/api/public/cmisVDM.py          | 31 ++++++-
 sonic_platform_base/sonic_xcvr/utils/cache.py | 38 +++++---
 tests/sonic_xcvr/test_cmisVDM.py              | 74 +++++++++++++++-
 tests/sonic_xcvr/test_cmis_cache.py           | 86 +++++++++++++++++++
 5 files changed, 217 insertions(+), 14 deletions(-)

diff --git a/sonic_platform_base/sonic_xcvr/api/public/cmis.py b/sonic_platform_base/sonic_xcvr/api/public/cmis.py
index 9e0784a45..0f09ecd78 100644
--- a/sonic_platform_base/sonic_xcvr/api/public/cmis.py
+++ b/sonic_platform_base/sonic_xcvr/api/public/cmis.py
@@ -137,6 +137,8 @@ def set_cache_enabled(cls, enabled: bool):
     def __init__(self, xcvr_eeprom, init_cdb_fw_handler=False):
         super(CmisApi, self).__init__(xcvr_eeprom)
         self.vdm = CmisVdmApi(xcvr_eeprom) if not self.is_flat_memory() else None
+        if self.vdm is not None:
+            self.vdm.cache_enabled = getattr(self, 'cache_enabled', False)
         self._init_cdb_fw_handler = init_cdb_fw_handler
         self._cdb_fw_hdlr = None
         self._cdb_mem_map = CdbMemMap(CdbCodes) if init_cdb_fw_handler else None
diff --git a/sonic_platform_base/sonic_xcvr/api/public/cmisVDM.py b/sonic_platform_base/sonic_xcvr/api/public/cmisVDM.py
index e12f7fef9..ae95a6fe3 100644
--- a/sonic_platform_base/sonic_xcvr/api/public/cmisVDM.py
+++ b/sonic_platform_base/sonic_xcvr/api/public/cmisVDM.py
@@ -6,6 +6,7 @@
 
 from ...fields import consts
 from ..xcvr_api import XcvrApi
+from ...utils.cache import read_only_cached_api_return
 import struct
 import time
 
@@ -29,9 +30,35 @@ class CmisVdmApi(XcvrApi):
     VDM_OBSERVABLE_STATISTIC = 0x2  # Statistic (min/max/avg) observable types
     VDM_OBSERVABLE_ALL = 0x3        # Both basic and statistic
 
+    # Default caching disabled; CmisApi wires cache_enabled onto the instance.
+    cache_enabled = False
+
     def __init__(self, xcvr_eeprom):
         super(CmisVdmApi, self).__init__(xcvr_eeprom)
-    
+
+    @read_only_cached_api_return
+    def _read_vdm_descriptor_page(self, page):
+        '''
+        Read and cache a raw VDM descriptor page (0x20-0x23).
+
+        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.
+
+        Caching is handled by read_only_cached_api_return, keyed per descriptor
+        page: it respects the class-level cache_enabled flag and only memoizes a
+        non-empty read, so a transient read failure is retried rather than
+        cached. The cache lives on the api object, which xcvrd recreates on
+        module re-insertion, so it is naturally invalidated when the module
+        changes.
+        '''
+        offset = page * PAGE_SIZE + PAGE_OFFSET
+        return self.xcvr_eeprom.read_raw(offset, PAGE_SIZE)
+
     def get_F16(self, value):
         '''
         This function converts raw data to "F16" format defined in cmis.
@@ -69,7 +96,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 {}
 
diff --git a/sonic_platform_base/sonic_xcvr/utils/cache.py b/sonic_platform_base/sonic_xcvr/utils/cache.py
index 21c8bbfd3..c97102de1 100644
--- a/sonic_platform_base/sonic_xcvr/utils/cache.py
+++ b/sonic_platform_base/sonic_xcvr/utils/cache.py
@@ -1,19 +1,35 @@
 from collections import abc
+import functools
 import os
 
 def read_only_cached_api_return(func):
-    """Cache until func() returns a non-None, non-empty collections cache_value."""
+    """Cache until func(...) returns a non-None, non-empty collections cache_value.
+
+    Works for methods with or without arguments. Results are cached per unique
+    set of positional and keyword arguments, so calls with different arguments
+    are cached independently. The per-method cache is stored on the instance
+    under ``_<func name>_cache`` as a dict keyed by the call arguments.
+    """
     cache_name = f'_{func.__name__}_cache'
-    def wrapper(self):
-        if not self.cache_enabled:
-            return func(self)
-        if not hasattr(self, cache_name):
-            cache_value = func(self)
-            setattr(self, cache_name, cache_value)
+
+    def _make_key(args, kwargs):
+        # Build a hashable key from the call arguments (excluding ``self``).
+        return (args, tuple(sorted(kwargs.items())))
+
+    @functools.wraps(func)
+    def wrapper(self, *args, **kwargs):
+        if not getattr(self, 'cache_enabled', False):
+            return func(self, *args, **kwargs)
+        cache = getattr(self, cache_name, None)
+        if cache is None:
+            cache = {}
+            setattr(self, cache_name, cache)
+        key = _make_key(args, kwargs)
+        if key not in cache:
+            cache[key] = func(self, *args, **kwargs)
         else:
-            cache_value = getattr(self, cache_name)
+            cache_value = cache[key]
             if cache_value is None or (isinstance(cache_value, abc.Iterable) and not cache_value):
-                cache_value = func(self)
-                setattr(self, cache_name, cache_value)
-        return cache_value
+                cache[key] = func(self, *args, **kwargs)
+        return cache[key]
     return wrapper
diff --git a/tests/sonic_xcvr/test_cmisVDM.py b/tests/sonic_xcvr/test_cmisVDM.py
index 890d03c8a..953d1acda 100644
--- a/tests/sonic_xcvr/test_cmisVDM.py
+++ b/tests/sonic_xcvr/test_cmisVDM.py
@@ -1,6 +1,7 @@
 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
@@ -112,6 +113,77 @@ 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
+            return bytearray(PAGE_SIZE)  # value / threshold pages
+
+        api.xcvr_eeprom.read_raw = read_raw
+        return api, reads
+
+    def test_vdm_descriptor_page_cached_when_enabled(self):
+        api, reads = self._make_vdm_api_with_counting_reader()
+        api.cache_enabled = True
+        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_page_not_cached_when_disabled(self):
+        api, reads = self._make_vdm_api_with_counting_reader()
+        api.cache_enabled = False
+        desc_off = 0x20 * PAGE_SIZE + PAGE_OFFSET
+        api.get_vdm_page(0x20, None)
+        api.get_vdm_page(0x20, None)
+        assert reads[desc_off] == 2
+
+    def test_vdm_descriptor_cache_is_per_page(self):
+        api, reads = self._make_vdm_api_with_counting_reader()
+        api.cache_enabled = True
+        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)
+        api.cache_enabled = True
+        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
+            return bytearray(PAGE_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
diff --git a/tests/sonic_xcvr/test_cmis_cache.py b/tests/sonic_xcvr/test_cmis_cache.py
index 4312fcaac..485689172 100755
--- a/tests/sonic_xcvr/test_cmis_cache.py
+++ b/tests/sonic_xcvr/test_cmis_cache.py
@@ -3,6 +3,54 @@
 from sonic_platform_base.sonic_xcvr.api.public.cmis import CmisApi
 from sonic_platform_base.sonic_xcvr.codes.public.sff8024 import Sff8024
 from sonic_platform_base.sonic_xcvr.fields import consts
+from sonic_platform_base.sonic_xcvr.utils.cache import read_only_cached_api_return
+
+
+class _ArgCacheTarget:
+    """Minimal target exercising read_only_cached_api_return with arguments."""
+    def __init__(self):
+        self.cache_enabled = True
+        self.reads = []
+
+    @read_only_cached_api_return
+    def read_page(self, page):
+        self.reads.append(page)
+        return self._page_value(page)
+
+
+class TestReadOnlyCacheDecoratorWithArgs:
+    def test_caches_per_argument(self):
+        target = _ArgCacheTarget()
+        target._page_value = lambda page: [page]
+        # Same arg cached; distinct args cached independently.
+        assert target.read_page(0x20) == [0x20]
+        assert target.read_page(0x20) == [0x20]
+        assert target.read_page(0x21) == [0x21]
+        assert target.read_page(0x21) == [0x21]
+        assert target.reads == [0x20, 0x21]
+
+    def test_empty_result_not_cached(self):
+        target = _ArgCacheTarget()
+        seq = {"n": 0}
+
+        # First read for the page returns empty, then a non-empty value.
+        def read_page(page):
+            v = [] if seq["n"] == 0 else [page]
+            seq["n"] += 1
+            return v
+        target._page_value = read_page
+        assert target.read_page(0x20) == []     # empty, retried next time
+        assert target.read_page(0x20) == [0x20]  # now cached
+        assert target.read_page(0x20) == [0x20]  # served from cache
+        assert seq["n"] == 2
+
+    def test_disabled_does_not_cache(self):
+        target = _ArgCacheTarget()
+        target.cache_enabled = False
+        target._page_value = lambda page: [page]
+        target.read_page(0x20)
+        target.read_page(0x20)
+        assert target.reads == [0x20, 0x20]
 
 class TestReadOnlyCacheDecorator:
     def setup_method(self):
@@ -216,3 +264,41 @@ def test_get_application_advertisement_not_cached(self):
         assert first == {}
         assert second == {}
         assert self.api.xcvr_eeprom.read.call_count == 2
+
+class TestVdmCacheWiring:
+    def setup_method(self):
+        # cache_enabled is global (class-level) state; snapshot it so these
+        # tests neither depend on nor leak the flag toggled by other tests.
+        self._orig_cache_enabled = CmisApi.cache_enabled
+
+    def teardown_method(self):
+        CmisApi.cache_enabled = self._orig_cache_enabled
+
+    @staticmethod
+    def _non_flat_eeprom():
+        # is_flat_memory() reads FLAT_MEM_FIELD; returning False marks the
+        # module as non-flat so CmisApi creates the vdm sub-API.
+        eeprom = MagicMock()
+        eeprom.read.return_value = False
+        return eeprom
+
+    def test_vdm_inherits_cache_enabled_true(self):
+        # vdm.cache_enabled is wired in __init__, so the flag must be set
+        # before construction -- matching how xcvrd enables caching.
+        CmisApi.set_cache_enabled(True)
+        api = CmisApi(self._non_flat_eeprom())
+        assert api.vdm is not None
+        assert api.vdm.cache_enabled is True
+
+    def test_vdm_inherits_cache_enabled_false(self):
+        CmisApi.set_cache_enabled(False)
+        api = CmisApi(self._non_flat_eeprom())
+        assert api.vdm is not None
+        assert api.vdm.cache_enabled is False
+
+    def test_vdm_cache_enabled_set_during_init(self):
+        # Setting the class attribute directly (no setter) is picked up at init.
+        CmisApi.cache_enabled = True
+        api = CmisApi(self._non_flat_eeprom())
+        assert api.vdm is not None
+        assert api.vdm.cache_enabled is True

From 98842047311451f66323b48601d41e17beb0ffb3 Mon Sep 17 00:00:00 2001
From: aditya-nexthop <aditya@nexthop.ai>
Date: Fri, 31 Jul 2026 22:14:02 +0000
Subject: [PATCH 2/4] cmisVDM: fix VDM descriptor cache tests to honor read
 size

The fake read_raw in the descriptor-cache tests ignored the requested
size and always returned a full 128-byte page. get_vdm_page reads the
value and threshold fields individually (2 bytes and 8 bytes), then
struct.unpack's them, so the oversized buffer raised

    struct.error: unpack requires a buffer of 2 bytes

failing all four descriptor-cache tests. Return exactly the requested
width instead. The cache assertions are unchanged.

Signed-off-by: aditya-nexthop <aditya@nexthop.ai>
---
 tests/sonic_xcvr/test_cmisVDM.py | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/tests/sonic_xcvr/test_cmisVDM.py b/tests/sonic_xcvr/test_cmisVDM.py
index 953d1acda..3c5c5f85e 100644
--- a/tests/sonic_xcvr/test_cmisVDM.py
+++ b/tests/sonic_xcvr/test_cmisVDM.py
@@ -128,8 +128,10 @@ 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
-            return bytearray(PAGE_SIZE)  # value / threshold pages
+                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
@@ -175,8 +177,8 @@ def read_raw(offset, size, *args, **kwargs):
             if page == 0x20:
                 v = seq[calls["n"]] if calls["n"] < len(seq) else descriptor
                 calls["n"] += 1
-                return v
-            return bytearray(PAGE_SIZE)
+                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 -> {}

From 0c94b17187b6bd4e18fbbb757d130ad771fd56d0 Mon Sep 17 00:00:00 2001
From: aditya-nexthop <aditya@nexthop.ai>
Date: Tue, 4 Aug 2026 17:53:05 +0000
Subject: [PATCH 3/4] cmisVDM: cache descriptor pages in a plain per-page dict

Address review feedback: cache the VDM descriptor pages in a
self._vdm_descriptor dict initialized in the constructor, instead of
extending the shared read_only_cached_api_return helper to support
arguments.

Signed-off-by: aditya-nexthop <aditya@nexthop.ai>
---
 .../sonic_xcvr/api/public/cmisVDM.py          | 25 ++++++----
 sonic_platform_base/sonic_xcvr/utils/cache.py | 38 +++++----------
 tests/sonic_xcvr/test_cmis_cache.py           | 48 -------------------
 3 files changed, 26 insertions(+), 85 deletions(-)

diff --git a/sonic_platform_base/sonic_xcvr/api/public/cmisVDM.py b/sonic_platform_base/sonic_xcvr/api/public/cmisVDM.py
index d9928f6cd..185c1e607 100644
--- a/sonic_platform_base/sonic_xcvr/api/public/cmisVDM.py
+++ b/sonic_platform_base/sonic_xcvr/api/public/cmisVDM.py
@@ -6,7 +6,6 @@
 
 from ...fields import consts
 from ..xcvr_api import XcvrApi
-from ...utils.cache import read_only_cached_api_return
 import struct
 import time
 
@@ -35,11 +34,13 @@ 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 = {}
 
-    @read_only_cached_api_return
     def _read_vdm_descriptor_page(self, page):
         '''
-        Read and cache a raw VDM descriptor page (0x20-0x23).
+        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
@@ -49,15 +50,19 @@ def _read_vdm_descriptor_page(self, page):
         reused across DOM cycles, removing one 128 B page read per descriptor
         page per port per cycle.
 
-        Caching is handled by read_only_cached_api_return, keyed per descriptor
-        page: it respects the class-level cache_enabled flag and only memoizes a
-        non-empty read, so a transient read failure is retried rather than
-        cached. The cache lives on the api object, which xcvrd recreates on
-        module re-insertion, so it is naturally invalidated when the module
-        changes.
+        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.
+        Honors cache_enabled, which CmisApi wires on from its own flag.
         '''
         offset = page * PAGE_SIZE + PAGE_OFFSET
-        return self.xcvr_eeprom.read_raw(offset, PAGE_SIZE)
+        if not self.cache_enabled:
+            return self.xcvr_eeprom.read_raw(offset, PAGE_SIZE)
+        if not self._vdm_descriptor.get(page):
+            self._vdm_descriptor[page] = self.xcvr_eeprom.read_raw(offset, PAGE_SIZE)
+        return self._vdm_descriptor[page]
 
     def get_F16(self, value):
         '''
diff --git a/sonic_platform_base/sonic_xcvr/utils/cache.py b/sonic_platform_base/sonic_xcvr/utils/cache.py
index c97102de1..21c8bbfd3 100644
--- a/sonic_platform_base/sonic_xcvr/utils/cache.py
+++ b/sonic_platform_base/sonic_xcvr/utils/cache.py
@@ -1,35 +1,19 @@
 from collections import abc
-import functools
 import os
 
 def read_only_cached_api_return(func):
-    """Cache until func(...) returns a non-None, non-empty collections cache_value.
-
-    Works for methods with or without arguments. Results are cached per unique
-    set of positional and keyword arguments, so calls with different arguments
-    are cached independently. The per-method cache is stored on the instance
-    under ``_<func name>_cache`` as a dict keyed by the call arguments.
-    """
+    """Cache until func() returns a non-None, non-empty collections cache_value."""
     cache_name = f'_{func.__name__}_cache'
-
-    def _make_key(args, kwargs):
-        # Build a hashable key from the call arguments (excluding ``self``).
-        return (args, tuple(sorted(kwargs.items())))
-
-    @functools.wraps(func)
-    def wrapper(self, *args, **kwargs):
-        if not getattr(self, 'cache_enabled', False):
-            return func(self, *args, **kwargs)
-        cache = getattr(self, cache_name, None)
-        if cache is None:
-            cache = {}
-            setattr(self, cache_name, cache)
-        key = _make_key(args, kwargs)
-        if key not in cache:
-            cache[key] = func(self, *args, **kwargs)
+    def wrapper(self):
+        if not self.cache_enabled:
+            return func(self)
+        if not hasattr(self, cache_name):
+            cache_value = func(self)
+            setattr(self, cache_name, cache_value)
         else:
-            cache_value = cache[key]
+            cache_value = getattr(self, cache_name)
             if cache_value is None or (isinstance(cache_value, abc.Iterable) and not cache_value):
-                cache[key] = func(self, *args, **kwargs)
-        return cache[key]
+                cache_value = func(self)
+                setattr(self, cache_name, cache_value)
+        return cache_value
     return wrapper
diff --git a/tests/sonic_xcvr/test_cmis_cache.py b/tests/sonic_xcvr/test_cmis_cache.py
index 485689172..051deb219 100755
--- a/tests/sonic_xcvr/test_cmis_cache.py
+++ b/tests/sonic_xcvr/test_cmis_cache.py
@@ -3,54 +3,6 @@
 from sonic_platform_base.sonic_xcvr.api.public.cmis import CmisApi
 from sonic_platform_base.sonic_xcvr.codes.public.sff8024 import Sff8024
 from sonic_platform_base.sonic_xcvr.fields import consts
-from sonic_platform_base.sonic_xcvr.utils.cache import read_only_cached_api_return
-
-
-class _ArgCacheTarget:
-    """Minimal target exercising read_only_cached_api_return with arguments."""
-    def __init__(self):
-        self.cache_enabled = True
-        self.reads = []
-
-    @read_only_cached_api_return
-    def read_page(self, page):
-        self.reads.append(page)
-        return self._page_value(page)
-
-
-class TestReadOnlyCacheDecoratorWithArgs:
-    def test_caches_per_argument(self):
-        target = _ArgCacheTarget()
-        target._page_value = lambda page: [page]
-        # Same arg cached; distinct args cached independently.
-        assert target.read_page(0x20) == [0x20]
-        assert target.read_page(0x20) == [0x20]
-        assert target.read_page(0x21) == [0x21]
-        assert target.read_page(0x21) == [0x21]
-        assert target.reads == [0x20, 0x21]
-
-    def test_empty_result_not_cached(self):
-        target = _ArgCacheTarget()
-        seq = {"n": 0}
-
-        # First read for the page returns empty, then a non-empty value.
-        def read_page(page):
-            v = [] if seq["n"] == 0 else [page]
-            seq["n"] += 1
-            return v
-        target._page_value = read_page
-        assert target.read_page(0x20) == []     # empty, retried next time
-        assert target.read_page(0x20) == [0x20]  # now cached
-        assert target.read_page(0x20) == [0x20]  # served from cache
-        assert seq["n"] == 2
-
-    def test_disabled_does_not_cache(self):
-        target = _ArgCacheTarget()
-        target.cache_enabled = False
-        target._page_value = lambda page: [page]
-        target.read_page(0x20)
-        target.read_page(0x20)
-        assert target.reads == [0x20, 0x20]
 
 class TestReadOnlyCacheDecorator:
     def setup_method(self):

From e604746950926154daf0a52b817c8e435ec3c5c0 Mon Sep 17 00:00:00 2001
From: aditya-nexthop <aditya@nexthop.ai>
Date: Tue, 4 Aug 2026 18:35:37 +0000
Subject: [PATCH 4/4] cmisVDM: always cache the descriptor pages, drop the
 cache flag

Address review feedback: there is no use case for not caching the VDM
descriptor pages, so remove the cache_enabled gate from CmisVdmApi
rather than defaulting it on.

Signed-off-by: aditya-nexthop <aditya@nexthop.ai>
---
 .../sonic_xcvr/api/public/cmis.py             |  2 -
 .../sonic_xcvr/api/public/cmisVDM.py          |  8 +---
 tests/sonic_xcvr/test_cmisVDM.py              | 24 ++++--------
 tests/sonic_xcvr/test_cmis_cache.py           | 38 -------------------
 4 files changed, 9 insertions(+), 63 deletions(-)

diff --git a/sonic_platform_base/sonic_xcvr/api/public/cmis.py b/sonic_platform_base/sonic_xcvr/api/public/cmis.py
index 91a6dc355..cecc9792d 100644
--- a/sonic_platform_base/sonic_xcvr/api/public/cmis.py
+++ b/sonic_platform_base/sonic_xcvr/api/public/cmis.py
@@ -137,8 +137,6 @@ def set_cache_enabled(cls, enabled: bool):
     def __init__(self, xcvr_eeprom, init_cdb_fw_handler=False):
         super(CmisApi, self).__init__(xcvr_eeprom)
         self.vdm = CmisVdmApi(xcvr_eeprom) if not self.is_flat_memory() else None
-        if self.vdm is not None:
-            self.vdm.cache_enabled = getattr(self, 'cache_enabled', False)
         self._init_cdb_fw_handler = init_cdb_fw_handler
         self._cdb_fw_hdlr = None
         self._cdb_mem_map = CdbMemMap(CdbCodes) if init_cdb_fw_handler else None
diff --git a/sonic_platform_base/sonic_xcvr/api/public/cmisVDM.py b/sonic_platform_base/sonic_xcvr/api/public/cmisVDM.py
index 185c1e607..de3ce54c9 100644
--- a/sonic_platform_base/sonic_xcvr/api/public/cmisVDM.py
+++ b/sonic_platform_base/sonic_xcvr/api/public/cmisVDM.py
@@ -29,9 +29,6 @@ class CmisVdmApi(XcvrApi):
     VDM_OBSERVABLE_STATISTIC = 0x2  # Statistic (min/max/avg) observable types
     VDM_OBSERVABLE_ALL = 0x3        # Both basic and statistic
 
-    # Default caching disabled; CmisApi wires cache_enabled onto the instance.
-    cache_enabled = False
-
     def __init__(self, xcvr_eeprom):
         super(CmisVdmApi, self).__init__(xcvr_eeprom)
         # Raw VDM descriptor pages, keyed by page. Populated lazily by
@@ -55,12 +52,9 @@ def _read_vdm_descriptor_page(self, page):
         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.
-        Honors cache_enabled, which CmisApi wires on from its own flag.
         '''
-        offset = page * PAGE_SIZE + PAGE_OFFSET
-        if not self.cache_enabled:
-            return self.xcvr_eeprom.read_raw(offset, PAGE_SIZE)
         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]
 
diff --git a/tests/sonic_xcvr/test_cmisVDM.py b/tests/sonic_xcvr/test_cmisVDM.py
index 4907484ea..409b35714 100644
--- a/tests/sonic_xcvr/test_cmisVDM.py
+++ b/tests/sonic_xcvr/test_cmisVDM.py
@@ -9,10 +9,13 @@
 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)
@@ -121,9 +124,8 @@ def read_raw(offset, size, *args, **kwargs):
         api.xcvr_eeprom.read_raw = read_raw
         return api, reads
 
-    def test_vdm_descriptor_page_cached_when_enabled(self):
+    def test_vdm_descriptor_page_cached(self):
         api, reads = self._make_vdm_api_with_counting_reader()
-        api.cache_enabled = True
         desc_off = 0x20 * PAGE_SIZE + PAGE_OFFSET
         val_off = 0x24 * PAGE_SIZE + PAGE_OFFSET
         for _ in range(3):
@@ -132,17 +134,8 @@ def test_vdm_descriptor_page_cached_when_enabled(self):
         assert reads[desc_off] == 1
         assert reads[val_off] == 3
 
-    def test_vdm_descriptor_page_not_cached_when_disabled(self):
-        api, reads = self._make_vdm_api_with_counting_reader()
-        api.cache_enabled = False
-        desc_off = 0x20 * PAGE_SIZE + PAGE_OFFSET
-        api.get_vdm_page(0x20, None)
-        api.get_vdm_page(0x20, None)
-        assert reads[desc_off] == 2
-
     def test_vdm_descriptor_cache_is_per_page(self):
         api, reads = self._make_vdm_api_with_counting_reader()
-        api.cache_enabled = True
         for page in (0x20, 0x21, 0x20, 0x21):
             api.get_vdm_page(page, None)
         assert reads[0x20 * PAGE_SIZE + PAGE_OFFSET] == 1
@@ -152,7 +145,6 @@ 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)
-        api.cache_enabled = True
         descriptor = tuple([16, 9] + [0] * (PAGE_SIZE - 2))
         seq = [None, descriptor, descriptor]  # first read fails, then succeeds
         calls = {"n": 0}
diff --git a/tests/sonic_xcvr/test_cmis_cache.py b/tests/sonic_xcvr/test_cmis_cache.py
index 051deb219..4312fcaac 100755
--- a/tests/sonic_xcvr/test_cmis_cache.py
+++ b/tests/sonic_xcvr/test_cmis_cache.py
@@ -216,41 +216,3 @@ def test_get_application_advertisement_not_cached(self):
         assert first == {}
         assert second == {}
         assert self.api.xcvr_eeprom.read.call_count == 2
-
-class TestVdmCacheWiring:
-    def setup_method(self):
-        # cache_enabled is global (class-level) state; snapshot it so these
-        # tests neither depend on nor leak the flag toggled by other tests.
-        self._orig_cache_enabled = CmisApi.cache_enabled
-
-    def teardown_method(self):
-        CmisApi.cache_enabled = self._orig_cache_enabled
-
-    @staticmethod
-    def _non_flat_eeprom():
-        # is_flat_memory() reads FLAT_MEM_FIELD; returning False marks the
-        # module as non-flat so CmisApi creates the vdm sub-API.
-        eeprom = MagicMock()
-        eeprom.read.return_value = False
-        return eeprom
-
-    def test_vdm_inherits_cache_enabled_true(self):
-        # vdm.cache_enabled is wired in __init__, so the flag must be set
-        # before construction -- matching how xcvrd enables caching.
-        CmisApi.set_cache_enabled(True)
-        api = CmisApi(self._non_flat_eeprom())
-        assert api.vdm is not None
-        assert api.vdm.cache_enabled is True
-
-    def test_vdm_inherits_cache_enabled_false(self):
-        CmisApi.set_cache_enabled(False)
-        api = CmisApi(self._non_flat_eeprom())
-        assert api.vdm is not None
-        assert api.vdm.cache_enabled is False
-
-    def test_vdm_cache_enabled_set_during_init(self):
-        # Setting the class attribute directly (no setter) is picked up at init.
-        CmisApi.cache_enabled = True
-        api = CmisApi(self._non_flat_eeprom())
-        assert api.vdm is not None
-        assert api.vdm.cache_enabled is True