Skip to content

Commit 5b32ad1

Browse files
jianyuewuFengPan-Frank
authored andcommitted
Support vendor-specific temperature adjustments (sonic-net#24957)
- Why I did it At 40°C ambient temperature with current FW+SW, some modules have >7.6% probability of reaching 75°C, which triggers false temperature warnings. This PR implements vendor-specific temperature threshold support to eliminate false warnings while maintaining accurate temperature telemetry for monitoring purposes. - How I did it Implemented new API for vendor-specific temperature offset adjustments: New API: Add get_vendor_info() API with caching support. Smart Module Detection: Cache vendor information (Manufacturer + Part Number) for each module. Skip redundant vendor info updates when the same module is replugged. - How to verify it Plug in optical module -> Verify vendor info sent to Nvidia API Unplug and replug same module -> Verify no redundant vendor info update. Replace with different module -> Verify new vendor info sent. Signed-off-by: Feng Pan <fenpan@microsoft.com>
1 parent ecae6e0 commit 5b32ad1

4 files changed

Lines changed: 261 additions & 12 deletions

File tree

platform/mellanox/mlnx-platform-api/sonic_platform/sfp.py

Lines changed: 83 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,19 @@
4545
except ImportError as e:
4646
raise ImportError (str(e) + "- required module not found")
4747

48+
try:
49+
import sys
50+
sys.path.append('/run/hw-management/bin')
51+
import hw_management_independent_mode_update
52+
except ImportError:
53+
# Only mock if running under pytest (check if pytest is imported)
54+
if 'pytest' in sys.modules:
55+
from unittest import mock
56+
hw_management_independent_mode_update = mock.MagicMock()
57+
hw_management_independent_mode_update.vendor_data_set_module = mock.MagicMock()
58+
else:
59+
raise
60+
4861
# Define the sdk constants
4962
SX_PORT_MODULE_STATUS_INITIALIZING = 0
5063
SX_PORT_MODULE_STATUS_PLUGGED = 1
@@ -426,6 +439,9 @@ def __init__(self, sfp_index, sfp_type=None, slot_id=0, linecard_port_count=0, l
426439
self.sn = None
427440
self.temp_high_threshold = None
428441
self.temp_critical_threshold = None
442+
self.retry_read_vendor = 5
443+
self.manufacturer = None
444+
self.part_number = None
429445

430446
def __str__(self):
431447
return f'SFP {self.sdk_index}'
@@ -867,19 +883,84 @@ def reinit_if_sn_changed(self):
867883
sn = self._get_serial()
868884
if sn != self.sn:
869885
self.reinit()
870-
self.sn = self._get_serial()
886+
# Clear cached vendor info so a new module will be re-read
887+
self.manufacturer = None
888+
self.part_number = None
871889
self.temp_high_threshold = None
872890
self.temp_critical_threshold = None
891+
self.sn = self._get_serial()
892+
if self.sn is not None:
893+
self.retry_read_vendor = 5
894+
else:
895+
self.retry_read_vendor = 0
873896
return True
874897
return False
875-
898+
899+
def get_vendor_info(self):
900+
"""Get SFP vendor info (manufacturer and part number).
901+
Reads fields via xcvr_eeprom to avoid manual offset logic.
902+
Uses cache to avoid redundant reads.
903+
Returns:
904+
tuple: (manufacturer, part_number) or (None, None) if read fails
905+
"""
906+
try:
907+
display_idx = self.sdk_index + 1
908+
if self.manufacturer is not None and self.part_number is not None:
909+
return self.manufacturer, self.part_number
910+
911+
api = self.get_xcvr_api()
912+
if not api or api.xcvr_eeprom is None:
913+
return None, None
914+
915+
try:
916+
manufacturer = api.xcvr_eeprom.read(consts.VENDOR_NAME_FIELD)
917+
part_number = api.xcvr_eeprom.read(consts.VENDOR_PART_NO_FIELD)
918+
logger.log_info(f"SFP {display_idx} vendor info read: manufacturer='{manufacturer}', part_number='{part_number}'")
919+
except Exception as e:
920+
logger.log_error(f"SFP {display_idx} vendor info read failed: {e}")
921+
manufacturer = None
922+
part_number = None
923+
924+
if manufacturer and part_number:
925+
self.manufacturer = manufacturer
926+
self.part_number = part_number
927+
return manufacturer, part_number
928+
929+
return None, None
930+
except Exception:
931+
return None, None
932+
876933
def get_temperature_info(self):
877934
"""Get SFP temperature info in a fast way. This function is faster than calling following functions one by one: get_temperature, get_temperature_warning_threshold, get_temperature_critical_threshold.
878935
879936
Returns:
880937
tuple: (temperature, warning_threshold, critical_threshold)
881938
"""
882939
try:
940+
sn_changed = self.reinit_if_sn_changed()
941+
if self.retry_read_vendor > 0:
942+
try:
943+
manufacturer, part_number = self.get_vendor_info()
944+
if manufacturer and part_number:
945+
vendor_info = {'manufacturer': manufacturer, 'part_number': part_number}
946+
hw_management_independent_mode_update.vendor_data_set_module(
947+
0, # ASIC index always 0 for now
948+
self.sdk_index + 1,
949+
vendor_info
950+
)
951+
logger.log_notice(f'Module {self.sdk_index + 1} vendor info updated - '
952+
f'manufacturer: {manufacturer} part_number: {part_number}')
953+
self.retry_read_vendor = 0
954+
else:
955+
self.retry_read_vendor -= 1
956+
if self.retry_read_vendor == 0:
957+
logger.log_notice(f"SFP {self.sdk_index + 1}: vendor info unavailable after retries")
958+
except Exception as e:
959+
logger.log_warning(f'Failed to publish vendor info for SFP {self.sdk_index + 1} - {e}')
960+
self.retry_read_vendor -= 1
961+
if self.retry_read_vendor == 0:
962+
logger.log_notice(f"SFP {self.sdk_index + 1}: vendor info unavailable after retries")
963+
883964
sw_control = self.is_sw_control()
884965
if not sw_control:
885966
return sw_control, None, None, None

platform/mellanox/mlnx-platform-api/sonic_platform/thermal_updater.py

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -27,14 +27,17 @@
2727
try:
2828
import hw_management_independent_mode_update
2929
except ImportError:
30-
# For unit test only
31-
from unittest import mock
32-
hw_management_independent_mode_update = mock.MagicMock()
33-
hw_management_independent_mode_update.module_data_set_module_counter = mock.MagicMock()
34-
hw_management_independent_mode_update.thermal_data_set_asic = mock.MagicMock()
35-
hw_management_independent_mode_update.thermal_data_set_module = mock.MagicMock()
36-
hw_management_independent_mode_update.thermal_data_clean_asic = mock.MagicMock()
37-
hw_management_independent_mode_update.thermal_data_clean_module = mock.MagicMock()
30+
# Only mock if running under pytest (check if pytest is imported)
31+
if 'pytest' in sys.modules:
32+
from unittest import mock
33+
hw_management_independent_mode_update = mock.MagicMock()
34+
hw_management_independent_mode_update.module_data_set_module_counter = mock.MagicMock()
35+
hw_management_independent_mode_update.thermal_data_set_asic = mock.MagicMock()
36+
hw_management_independent_mode_update.thermal_data_set_module = mock.MagicMock()
37+
hw_management_independent_mode_update.thermal_data_clean_asic = mock.MagicMock()
38+
hw_management_independent_mode_update.thermal_data_clean_module = mock.MagicMock()
39+
else:
40+
raise
3841

3942

4043
SFP_TEMPERATURE_SCALE = 1000

platform/mellanox/mlnx-platform-api/tests/test_sfp.py

Lines changed: 122 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -608,14 +608,134 @@ def mock_read(field):
608608
sfp.is_sw_control.side_effect = Exception('')
609609
assert sfp.get_temperature_info() == (False, None, None, None)
610610

611+
@mock.patch('time.sleep', mock.MagicMock())
612+
def test_get_temperature_info_vendor_retry_loop(self):
613+
sfp = SFP(0)
614+
sfp.reinit_if_sn_changed = mock.MagicMock(side_effect=[True, False, False])
615+
sfp.is_sw_control = mock.MagicMock(return_value=False)
616+
sfp.retry_read_vendor = 5
617+
# First two attempts fail, third succeeds
618+
sfp.get_vendor_info = mock.MagicMock(side_effect=[(None, None), (None, None), ('Mellanox', 'PN-9999')])
619+
620+
# Attempt 1: reinit sets retry counter, first vendor read fails (counter -> 4)
621+
sfp.get_temperature_info()
622+
# Attempt 2: retry counter >0, second vendor read fails (counter -> 3)
623+
sfp.get_temperature_info()
624+
# Attempt 3: retry counter >0, vendor read succeeds, counter cleared
625+
sfp.get_temperature_info()
626+
627+
assert sfp.get_vendor_info.call_count == 3
628+
assert sfp.retry_read_vendor == 0
629+
611630
def test_reinit_if_sn_changed(self):
612631
sfp = SFP(0)
613632
sfp.get_xcvr_api = mock.MagicMock(return_value=None)
614633
assert not sfp.reinit_if_sn_changed()
615-
634+
616635
sfp.get_xcvr_api.return_value = mock.MagicMock()
617636
sfp.get_xcvr_api.return_value.xcvr_eeprom.read = mock.MagicMock(return_value='1234567890')
618637
assert sfp.reinit_if_sn_changed()
619-
638+
assert sfp.retry_read_vendor == 5
639+
620640
sfp.get_xcvr_api.return_value.xcvr_eeprom.read.return_value = '1234567891'
621641
assert sfp.reinit_if_sn_changed()
642+
assert sfp.retry_read_vendor == 5
643+
644+
# Vendor cache should reset on reinit to allow new modules to be read
645+
sfp.sn = 'old_sn'
646+
sfp.manufacturer = 'OldVendor'
647+
sfp.part_number = 'OldPart'
648+
sfp._get_serial = mock.MagicMock(return_value='new_sn')
649+
assert sfp.reinit_if_sn_changed()
650+
assert sfp.manufacturer is None
651+
assert sfp.part_number is None
652+
assert sfp.retry_read_vendor == 5
653+
654+
@mock.patch('time.sleep', mock.MagicMock())
655+
def test_get_vendor_info_success_and_cache(self):
656+
sfp = SFP(0)
657+
mock_api = mock.MagicMock()
658+
mock_eeprom = mock.MagicMock()
659+
mock_api.xcvr_eeprom = mock_eeprom
660+
sfp.get_xcvr_api = mock.MagicMock(return_value=mock_api)
661+
662+
from sonic_platform_base.sonic_xcvr.fields import consts
663+
def mock_read(field):
664+
if field == consts.VENDOR_NAME_FIELD:
665+
return 'Mellanox'
666+
if field == consts.VENDOR_PART_NO_FIELD:
667+
return 'PN-1234'
668+
return None
669+
mock_eeprom.read.side_effect = mock_read
670+
671+
# First call reads from eeprom
672+
manufacturer, part_number = sfp.get_vendor_info()
673+
assert manufacturer == 'Mellanox'
674+
assert part_number == 'PN-1234'
675+
assert mock_eeprom.read.call_count == 2
676+
677+
# Second call should return cached values without additional reads
678+
manufacturer, part_number = sfp.get_vendor_info()
679+
assert manufacturer == 'Mellanox'
680+
assert part_number == 'PN-1234'
681+
assert mock_eeprom.read.call_count == 2
682+
683+
@mock.patch('time.sleep', mock.MagicMock())
684+
def test_get_vendor_info_retry_then_success(self):
685+
sfp = SFP(0)
686+
mock_api = mock.MagicMock()
687+
mock_eeprom = mock.MagicMock()
688+
mock_api.xcvr_eeprom = mock_eeprom
689+
sfp.get_xcvr_api = mock.MagicMock(return_value=mock_api)
690+
691+
from sonic_platform_base.sonic_xcvr.fields import consts
692+
state = {'fail_reads': 2}
693+
def flaky_read(field):
694+
if state['fail_reads'] > 0:
695+
state['fail_reads'] -= 1
696+
raise Exception('EEPROM not ready')
697+
if field == consts.VENDOR_NAME_FIELD:
698+
return 'Mellanox'
699+
if field == consts.VENDOR_PART_NO_FIELD:
700+
return 'PN-5678'
701+
return None
702+
mock_eeprom.read.side_effect = flaky_read
703+
704+
# First call fails (counter decremented)
705+
manufacturer, part_number = sfp.get_vendor_info()
706+
assert (manufacturer, part_number) == (None, None)
707+
# Second call fails (counter decremented)
708+
manufacturer, part_number = sfp.get_vendor_info()
709+
assert (manufacturer, part_number) == (None, None)
710+
# Third call succeeds (reads both fields)
711+
manufacturer, part_number = sfp.get_vendor_info()
712+
assert (manufacturer, part_number) == ('Mellanox', 'PN-5678')
713+
# Total read invocations: first two calls each raise on first field (2),
714+
# third call reads both fields (2) → 4 total
715+
assert mock_eeprom.read.call_count == 4
716+
717+
@mock.patch('time.sleep', mock.MagicMock())
718+
def test_get_vendor_info_all_fail(self):
719+
sfp = SFP(0)
720+
mock_api = mock.MagicMock()
721+
mock_eeprom = mock.MagicMock()
722+
mock_api.xcvr_eeprom = mock_eeprom
723+
sfp.get_xcvr_api = mock.MagicMock(return_value=mock_api)
724+
mock_eeprom.read.side_effect = Exception('EEPROM error')
725+
726+
manufacturer, part_number = sfp.get_vendor_info()
727+
assert manufacturer is None
728+
assert part_number is None
729+
730+
def test_get_vendor_info_no_api_or_missing_attr(self):
731+
sfp = SFP(0)
732+
# No API
733+
sfp.get_xcvr_api = mock.MagicMock(return_value=None)
734+
assert sfp.get_vendor_info() == (None, None)
735+
736+
# API without xcvr_eeprom attribute
737+
class DummyApi(object):
738+
pass
739+
sfp.get_xcvr_api.return_value = DummyApi()
740+
assert sfp.get_vendor_info() == (None, None)
741+

platform/mellanox/mlnx-platform-api/tests/test_thermal_updater.py

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,3 +109,48 @@ def test_update_module(self):
109109
hw_management_independent_mode_update.reset_mock()
110110
updater.update_module()
111111
hw_management_independent_mode_update.thermal_data_set_module.assert_called_once_with(0, 11, 0, 0, 0, 0)
112+
113+
# ---- SFP.get_temperature_info publishes vendor info on module change ----
114+
def _make_sfp_for_publish(self, sn_changed=True, vendor=('Innolight', 'TR-iQ13L-NVS')):
115+
# Import locally to avoid any potential name resolution issues in test scope
116+
from sonic_platform.sfp import SFP as _SFP
117+
sfp = object.__new__(_SFP)
118+
sfp.sdk_index = 10
119+
sfp.retry_read_vendor = 5 if sn_changed else 0
120+
sfp.is_sw_control = mock.MagicMock(return_value=True)
121+
sfp.reinit_if_sn_changed = mock.MagicMock(return_value=sn_changed)
122+
if vendor is None:
123+
sfp.get_vendor_info = mock.MagicMock(return_value=(None, None))
124+
else:
125+
sfp.get_vendor_info = mock.MagicMock(return_value=vendor)
126+
api = mock.MagicMock()
127+
api.get_transceiver_thresholds_support = mock.MagicMock(return_value=False)
128+
sfp.get_xcvr_api = mock.MagicMock(return_value=api)
129+
return sfp
130+
131+
def test_sfp_get_temperature_info_publishes_vendor_on_sn_change(self):
132+
from sonic_platform.sfp import hw_management_independent_mode_update as sfp_hw_management_independent_mode_update
133+
sfp = self._make_sfp_for_publish(sn_changed=True, vendor=('Innolight', 'TR-iQ13L-NVS'))
134+
with mock.patch('sonic_platform.sfp.SfpOptoeBase.get_temperature', return_value=55.0):
135+
sfp_hw_management_independent_mode_update.reset_mock()
136+
sfp.get_temperature_info()
137+
sfp_hw_management_independent_mode_update.vendor_data_set_module.assert_called_once_with(
138+
0, 11, {'manufacturer': 'Innolight', 'part_number': 'TR-iQ13L-NVS'}
139+
)
140+
141+
def test_sfp_get_temperature_info_no_publish_when_no_change(self):
142+
from sonic_platform.sfp import hw_management_independent_mode_update as sfp_hw_management_independent_mode_update
143+
sfp = self._make_sfp_for_publish(sn_changed=False, vendor=('Innolight', 'TR-iQ13L-NVS'))
144+
with mock.patch('sonic_platform.sfp.SfpOptoeBase.get_temperature', return_value=55.0):
145+
sfp_hw_management_independent_mode_update.reset_mock()
146+
sfp.get_temperature_info()
147+
sfp_hw_management_independent_mode_update.vendor_data_set_module.assert_not_called()
148+
149+
def test_sfp_get_temperature_info_no_publish_when_vendor_missing(self):
150+
from sonic_platform.sfp import hw_management_independent_mode_update as sfp_hw_management_independent_mode_update
151+
sfp = self._make_sfp_for_publish(sn_changed=True, vendor=None)
152+
with mock.patch('sonic_platform.sfp.SfpOptoeBase.get_temperature', return_value=55.0):
153+
sfp_hw_management_independent_mode_update.reset_mock()
154+
sfp.get_temperature_info()
155+
sfp_hw_management_independent_mode_update.vendor_data_set_module.assert_not_called()
156+

0 commit comments

Comments
 (0)