Independent Module - platform-daemon code to support port SI configuration per speed #3
Conversation
…dent Module feature
sonic-xcvrd/xcvrd/xcvrd.py
Outdated
| if type(api) == CmisApi: | ||
| appl_adv_dict = api.get_application_advertisement() | ||
| if speed_index is not None: | ||
| lane_speed_key = LANE_SPEED_KEY_PREFIX + (appl_adv_dict[speed_index].get('host_electrical_interface_id')).split()[0] |
There was a problem hiding this comment.
is it possible that the get call would return None?
There was a problem hiding this comment.
The 'host_electrical_interface_id' key is searched for in a dictionary returned from the EEPROM. Therefore, it is highly unlikely for this key not to be present in appl_adv_dict[speed_index] (returned from get_application_advertisement()).
Just to be on the safe side, I added some protection in case an invalid dictionary was returned from the EEPROM.
sonic-xcvrd/xcvrd/xcvrd.py
Outdated
| if not g_dict: | ||
| return | ||
|
|
||
| port_speed, lane_count = get_speed_and_lane_count(logical_port_name, cfg_port_tbl) |
There was a problem hiding this comment.
should we continue if port_speed or lane_count is -1?
There was a problem hiding this comment.
if port_speed or lane_count have invalid values, then lane_speed_key will be None, and therefore, if media_settings.json supports SI values per speed, then get_media_settings_value() will return an empty result, according to the updated flow of the parser. So in the case of invalid port_speed or lane_count, nothing will be posted to APP_DB during notify_media_settings().
sonic-xcvrd/xcvrd/xcvrd.py
Outdated
| # Stage custom SI settings | ||
| if optics_si_parser.optics_si_present(): | ||
| optics_si_dict = {} | ||
| # Apply module SI settings if applicable | ||
| lane_speed = int(speed/1000)//host_lane_count | ||
| optics_si_dict = optics_si_parser.fetch_optics_si_setting(pport, lane_speed, sfp) | ||
|
|
||
|
|
||
| helper_logger.log_debug("SI values for the connected module found in optics_si_settings.json:") |
There was a problem hiding this comment.
are you going to keep this debug code?
There was a problem hiding this comment.
I updated this log message according to Moshe's suggestion
|
@tshalvi please point to the HLD in PR description? |
…ing xcvrd accordingly
…function check_port_in_range() from media_settings_parser back to xcvrd as optics_si_parser also uses it
…_supported() and test_get_interface_speed())
…s in media_settings_parser
… media_settings.json, from leg_error to log_info
Description
I have updated the lookup in media_settings.json. With the updated parser, media_settings.json now supports different SI values for various lane speeds. Previously, get_media_settings_key() returned two types of keys: vendor_key and media_key. With these code changes, three keys are now returned: vendor_key, media_key, and lane_speed_key. The parser, implemented in get_media_settings_value(), intelligently utilizes the lane_speed_key only for JSON files that support per-speed port SI values. In cases where the JSON does not support this feature, the parser's functionality remains unchanged.
Motivation and Context
To achieve optimal configuration of their SerDes, some vendors wish to have the option to configure SerDes SI based on lane speeds. Therefore, we want to enhance the current ASIC configuration to provide support for configurations that take into account lane speed.
How Has This Been Tested?
I was running the existing unit tests to test these changes, and updated them if needed. They all passed.
Soon I will add new unit test to cover all my changes.
Additional Information (Optional)