[AMD-SMI] AI-NIC Cleanup

projects/amdsmi/amdsmi_cli/amdsmi_commands.py

@@ -564,8 +564,6 @@ def list_switchs(self, args):
             return False
         if args.switch == None:
             args.switch = self.device_handles_switchs
-            if isinstance(args.switch, list):
-                switchCount = len(args.switch)
             return False
         if isinstance(args.switch, list):
             switchCount = len(args.switch)
@@ -598,8 +596,6 @@ def list(self, args, multiple_devices=False, gpu=None, nic=None, switch=None):
             args.switch = switch 
 
         gpuCount = 0
-        nicCount = 0
-        switchCount = 0
 
         # Handle No GPU passed
         if args.gpu == None:
@@ -633,7 +629,7 @@ def list(self, args, multiple_devices=False, gpu=None, nic=None, switch=None):
         if self.helpers.is_ainic_initialized() or self.helpers.is_brcm_nic_initialized():
             nics,ainics = self._get_nics_from_args(args)
             if len(nics) > 0:
-                self.list_brcm_nic(args, False, nic=ainics)
+                self.list_brcm_nic(args, False, nic=nics)
             if len(ainics) > 0:
                 self.list_ainic(args, False, nic=ainics)
 
@@ -1738,20 +1734,20 @@ def _static_ainic(self, args, multiple_devices=False, nic=None):
         if args.nic:
             try:
                 nic_info = amdsmi_interface.amdsmi_get_ainic_info(args.nic, True)
-                filter = []
+                info_filter = []
                 if hasattr(args, "asic") and getattr(args, "asic"):
-                    filter.append("asic")
+                    info_filter.append("asic")
                 if hasattr(args, "bus") and getattr(args, "bus"):
-                    filter.append("bus")
+                    info_filter.append("bus")
                 if hasattr(args, "driver") and getattr(args, "driver"):
-                    filter.append("driver")
+                    info_filter.append("driver")
                 if hasattr(args, "numa") and getattr(args, "numa"):
-                    filter.append("numa")
-                if len(filter) == 0 or len(filter) == 4:
+                    info_filter.append("numa")
+                if len(info_filter) == 0 or len(info_filter) == 4:
                     static_dict["nic"] = nic_info
                 else:
                     nic_info_filtered = {}
-                    for attr in filter: #remove all attributes except the one in filter:
+                    for attr in info_filter: #remove all attributes except the one in info_filter:
                         nic_info_filtered = nic_info_filtered | {key: value for key, value in nic_info.items() if key.lower() == attr}
                     static_dict["nic"] = nic_info_filtered
             except amdsmi_exception.AmdSmiLibraryException as e:
@@ -1951,8 +1947,6 @@ def firmware_nic(self, args, multiple_devices=False, nic=None, fw_list=True):
             except amdsmi_exception.AmdSmiLibraryException as e:
                 logging.debug("Failed to get firmware info for nic %s | %s", nic_id, e.get_error_info())
 
-        multiple_devices_csv_override = False
-
         self.logger.store_nic_output(args.nic, 'values', fw_info)
 
         if multiple_devices:
@@ -4714,9 +4708,9 @@ def topology_nic(self, args, multiple_devices=False, gpu=None, nic=None,
             self.helpers.check_required_groups()
             self.group_check_printed = True
 
-        isSingleNICRequest = False #-N option
-        isSingleSwitchRequest= False #-bs option
-        isSingleGPURequest = False #-g option
+        is_single_nic_request = False #-N option
+        is_single_switch_request = False #-bs option
+        is_single_gpu_request = False #-g option
 
         gpucount = 0
         niccount = 0
@@ -4727,24 +4721,24 @@ def topology_nic(self, args, multiple_devices=False, gpu=None, nic=None,
         if not isinstance(args.nic, list):
             args.nic = [args.nic]
         if len(args.nic) == 1:
-            isSingleNICRequest = True
+            is_single_nic_request = True
         niccount = len(args.nic)
 
-        if args.switch == None:
+        if args.switch is None:
             args.switch = self.device_handles_switchs
         if not isinstance(args.switch, list):
             args.switch = [args.switch]
         if len(args.switch) == 1:
-            isSingleSwitchRequest = True
+            is_single_switch_request = True
         switchcount = len(args.switch)
 
-        if args.gpu == None:
+        if args.gpu is None:
             args.gpu = self.device_handles
         if not isinstance(args.gpu, list):
             args.gpu = [args.gpu]
         if len(args.gpu) == 1:
-            isSingleGPURequest = True
-        gpucount = len(args.switch)
+            is_single_gpu_request = True
+        gpucount = len(args.gpu)
 
         # Clear the table header
         self.logger.table_header = ''.rjust(12)
@@ -4768,7 +4762,10 @@ def topology_nic(self, args, multiple_devices=False, gpu=None, nic=None,
                 for gpu_dest in args.gpu:
                     gpu_bdf = amdsmi_interface.amdsmi_get_gpu_device_bdf(gpu_dest)
                     gpu_id = self.helpers.get_gpu_id_from_device_handle(gpu_dest)
-                    status = amdsmi_interface.amdsmi_get_nic_gpu_topo_info(dest_nic,gpu_dest)
+                    try:
+                        status = amdsmi_interface.amdsmi_get_nic_gpu_topo_info(dest_nic, gpu_dest)
+                    except amdsmi_exception.AmdSmiLibraryException as e:
+                        status = "N/A"
                     gpu_statuses_for_nic.append((gpu_bdf, status))  # Store BDF and status as tuple
 
                 # Store NIC BDF and associated GPU statuses in the dictionary
@@ -4798,7 +4795,7 @@ def topology_nic(self, args, multiple_devices=False, gpu=None, nic=None,
 
             # Add NIC rows with their associated GPU statuses
             for idx, (nic_bdf, gpu_info) in enumerate(topo_dict.items()):
-                if not isSingleNICRequest:
+                if not is_single_nic_request:
                     if self.logger.is_human_readable_format():
                         nic_row = {'brcm_nic': f"BRCM_NIC{idx}".ljust(12), 'bdf': f"{nic_bdf}".ljust(18) }
                     else:
@@ -4860,15 +4857,15 @@ def topology_nic(self, args, multiple_devices=False, gpu=None, nic=None,
             if self.logger.is_human_readable_format():
                 tabular_output.append(header_row)
 
-            if isSingleNICRequest:
+            if is_single_nic_request:
                 gpucount = 0
                 niccount = 1
                 switchcount = 0
-            if isSingleSwitchRequest:
+            if is_single_switch_request:
                 gpucount = 0
                 niccount = 0
                 switchcount = 1
-            if isSingleGPURequest:
+            if is_single_gpu_request:
                 gpucount = 1
                 niccount = 0
                 switchcount = 0
@@ -7144,12 +7141,12 @@ def monitor(self, args, multiple_devices=False, watching_output=False, gpu=None,
         if process:
             args.process = process
         if brcm_nic or args.brcm_nic:
-            self.monitor_nic(args, multiple_devices, watching_output, args.nic, watch, watch_time, iterations,
-                            args.temperature, args.brcm_nic)
+            self.metric_nic(args, multiple_devices, watching_output, watch, watch_time, iterations,
+                            nic=args.nic, nic_temperature=args.temperature)
             return
         if brcm_switch or args.brcm_switch:
-            self.monitor_switch(args, multiple_devices, watching_output, args.switch, watch, watch_time, iterations,
-                            args.pcie, args.brcm_switch)
+            self.metric_switch(args, multiple_devices, watching_output, watch, watch_time, iterations,
+                            switch=args.switch)
             return
         if not self.helpers.is_virtual_os():
             if violation:

projects/amdsmi/amdsmi_cli/amdsmi_helpers.py

@@ -350,7 +350,7 @@ def get_gpu_choices(self):
         device_handles = []
 
         try:
-            # amdsmi_get_processor_handles returns the device_handles storted for gpu_id
+            # amdsmi_get_processor_handles returns the device_handles sorted for gpu_id
             device_handles = amdsmi_interface.amdsmi_get_processor_handles()
         except amdsmi_interface.AmdSmiLibraryException as e:
             if e.err_code in (amdsmi_interface.amdsmi_wrapper.AMDSMI_STATUS_NOT_INIT,
@@ -388,8 +388,6 @@ def get_gpu_choices(self):
 
     def nic_choices_from_nic_info(self, nic_info, nic_id, device_handle, max_padding, nic_choices, nic_choices_str):
         bdf = nic_info['bdf']
-
-        #uuid="abc"
         uuid = nic_info['UUID']
 
         nic_choices[str(nic_id)] = {
@@ -412,7 +410,7 @@ def get_nic_choices(self):
         ainic_device_handles = []
 
         try:
-            # get_nic_handles returns the device_handles storted for nic_id
+            # get_nic_handles returns the device_handles sorted for nic_id
             nic_device_handles = amdsmi_interface.get_nic_handles()
             ainic_device_handles = amdsmi_interface.get_ainic_handles()
 
@@ -452,7 +450,7 @@ def get_switch_choices(self):
         device_handles = []
 
         try:
-            # get_switch_handles returns the device_handles storted for switch_id
+            # get_switch_handles returns the device_handles sorted for switch_id
             device_handles = amdsmi_interface.get_switch_handles()
 
         except amdsmi_interface.AmdSmiLibraryException as e:
@@ -472,8 +470,6 @@ def get_switch_choices(self):
 
             for switch_id, device_handle in enumerate(device_handles):
                 bdf = amdsmi_interface.amdsmi_get_switch_device_bdf(device_handle)
-
-                #uuid="abc"
                 uuid = amdsmi_interface.amdsmi_get_switch_device_uuid(device_handle)
 
                 switch_choices[str(switch_id)] = {
@@ -599,9 +595,6 @@ def get_device_handles_from_nic_selections(self, nic_selections: List[str], nic_
 
                 # Check if passed nic is a nic ID or UUID
                 if nic_selection == nic_id or nic_selection.lower() == uuid:
-
-                    device_type=amdsmi_interface.amdsmi_get_processor_type(device_handle)
-
                     selected_device_handles.append(device_handle)
                     valid_nic_choice = True
                     break
@@ -612,7 +605,7 @@ def get_device_handles_from_nic_selections(self, nic_selections: List[str], nic_
                         break
 
             if not valid_nic_choice:
-                logging.debug(f"AMDSMIHelpers.get_device_handles_from_gpu_selections - Unable to convert {nic_selection}")
+                logging.debug(f"AMDSMIHelpers.get_device_handles_from_nic_selections - Unable to convert {nic_selection}")
 
                 return False, nic_selection
 
@@ -627,7 +620,7 @@ def get_device_handles_from_switch_selections(self, switch_selections: List[str]
             Args:
                 switch_selections (list[str]): Selected switch ID(s), BDF(s), or UUID(s):
                         ex: ID:0  | BDF:0000:23:00.0 | UUID:ffffffff-0000-1000-0000-000000000000
-                switch_choices (dict{switch_choices}): This is a dictionary of the possible gpu_choices
+                switch_choices (dict{switch_choices}): This is a dictionary of the possible switch_choices
             Returns:
                 (True, list[device_handles]): Returns a list of all the switch_selections converted to
                     amdsmi device_handles
@@ -654,9 +647,6 @@ def get_device_handles_from_switch_selections(self, switch_selections: List[str]
 
                     # Check if passed switch is a switch ID or UUID
                     if switch_selection == switch_id or switch_selection.lower() == uuid:
-
-                        device_type=amdsmi_interface.amdsmi_get_processor_type(device_handle)
-
                         selected_device_handles.append(device_handle)
                         valid_switch_choice = True
                         break
@@ -671,7 +661,7 @@ def get_device_handles_from_switch_selections(self, switch_selections: List[str]
                             pass
 
                 if not valid_switch_choice:
-                    logging.debug(f"AMDSMIHelpers.get_device_handles_from_gpu_selections - Unable to convert {switch_selection}")
+                    logging.debug(f"AMDSMIHelpers.get_device_handles_from_switch_selections - Unable to convert {switch_selection}")
 
                     return False, switch_selection
 

projects/amdsmi/amdsmi_cli/amdsmi_logger.py

@@ -471,7 +471,7 @@ def store_ainic_output(self, device_handle, argument, data):
         self._store_ainic_output_amdsmi(nic_id=nic_id, argument=argument, data=data)
 
     def store_switch_output(self, device_handle, argument, data):
-        """ Convert device handle to nic id and store output
+        """ Convert device handle to switch id and store output
             params:
                 device_handle - device handle object to the target device output
                 argument (str) - key to store data

projects/amdsmi/amdsmi_cli/amdsmi_parser.py

@@ -510,7 +510,7 @@ def _nic_select(self, nic_choices):
 
         amdsmi_helpers = self.helpers
         class _NICSelectAction(argparse.Action):
-            ouputformat=self.helpers.get_output_format()
+            output_format=self.helpers.get_output_format()
             # Checks the values
             def __call__(self, parser, args, values, option_string=None):
                 if "all" in nic_choices:
@@ -521,9 +521,9 @@ def __call__(self, parser, args, values, option_string=None):
                     setattr(args, self.dest, selected_device_handles)
                 else:
                     if selected_device_handles == '':
-                        raise amdsmi_cli_exceptions.AmdSmiMissingParameterValueException("--nic", _NICSelectAction.ouputformat)
+                        raise amdsmi_cli_exceptions.AmdSmiMissingParameterValueException("--nic", _NICSelectAction.output_format)
                     else:
-                        raise amdsmi_cli_exceptions.AmdSmiDeviceNotFoundException(selected_device_handles, _NICSelectAction.ouputformat)
+                        raise amdsmi_cli_exceptions.AmdSmiDeviceNotFoundException(selected_device_handles, _NICSelectAction.output_format)
 
 
         return _NICSelectAction
@@ -537,8 +537,8 @@ def _switch_select(self, switch_choices):
         """
 
         amdsmi_helpers = self.helpers
-        class _switchSelectAction(argparse.Action):
-            ouputformat=self.helpers.get_output_format()
+        class _SwitchSelectAction(argparse.Action):
+            output_format=self.helpers.get_output_format()
             # Checks the values
             def __call__(self, parser, args, values, option_string=None):
                 if "all" in switch_choices:
@@ -549,12 +549,12 @@ def __call__(self, parser, args, values, option_string=None):
                     setattr(args, self.dest, selected_device_handles)
                 else:
                     if selected_device_handles == '':
-                        raise amdsmi_cli_exceptions.AmdSmiMissingParameterValueException("--switch", _switchSelectAction.ouputformat)
+                        raise amdsmi_cli_exceptions.AmdSmiMissingParameterValueException("--switch", _SwitchSelectAction.output_format)
                     else:
-                        raise amdsmi_cli_exceptions.AmdSmiDeviceNotFoundException(selected_device_handles, _switchSelectAction.ouputformat)
+                        raise amdsmi_cli_exceptions.AmdSmiDeviceNotFoundException(selected_device_handles, _SwitchSelectAction.output_format)
 
 
-        return _switchSelectAction
+        return _SwitchSelectAction
 
 
     def _cpu_select(self, cpu_choices):

projects/amdsmi/example/amd_smi_drm_example.cc

@@ -286,7 +286,7 @@ static const std::map<processor_type_t, std::string>
   {AMDSMI_PROCESSOR_TYPE_NON_AMD_CPU, "NON_AMD_CPU"},
   {AMDSMI_PROCESSOR_TYPE_AMD_CPU_CORE, "AMD_CPU_CORE"},
   {AMDSMI_PROCESSOR_TYPE_AMD_NIC, "AMD_AINIC"},
-  {AMDSMI_PROCESSOR_TYPE_BRCM_NIC, "BRCM_NIC,"},
+  {AMDSMI_PROCESSOR_TYPE_BRCM_NIC, "BRCM_NIC"},
   {AMDSMI_PROCESSOR_TYPE_BRCM_SWITCH, "BRCM_SWITCH"}
 };
 

projects/amdsmi/include/amd_smi/impl/amd_smi_system.h

@@ -36,7 +36,7 @@
 #ifdef BRCM_NIC
 #include "amd_smi/impl/nic/amd_smi_no_drm_nic.h"
 #include "amd_smi/impl/nic/amd_smi_no_drm_switch.h"
-#endif//BRCM_NIC
+#endif  // BRCM_NIC
 namespace amd::smi {
 
 // Singleton: Only one system in an application
@@ -88,9 +88,9 @@ class AMDSmiSystem {
     smi_nic_ctx_t ainic_ctx_;
     std::vector<AMDSmiAINICDevice::AINICInfo> ai_nic_info_;
 #ifdef BRCM_NIC
-    AMDSmiNoDrmNIC no_drm_nic;
-    AMDSmiNoDrmSwitch no_drm_switch;
-#endif//BRCM_NIC
+    AMDSmiNoDrmNIC no_drm_nic_;
+    AMDSmiNoDrmSwitch no_drm_switch_;
+#endif  // BRCM_NIC
     std::vector<AMDSmiSocket*> sockets_;
     std::set<AMDSmiProcessor*> processors_;     // Track valid processors
     std::set<AMDSmiProcessor*> nic_processors_;  // Track valid nic processors

projects/amdsmi/include/amd_smi/impl/nic/amd_smi_lspci_commands.h

@@ -31,7 +31,7 @@
 #include "amd_smi/amdsmi.h"
 #include "rocm_smi/rocm_smi_logger.h"
 
-amdsmi_status_t get_lspci_device_data(std::string bdfStr, std::string search_key, std::string &version);
-amdsmi_status_t get_lspci_root_switch(amdsmi_bdf_t devicehBdf, amdsmi_bdf_t *switchBdf);
+amdsmi_status_t get_lspci_device_data(std::string bdf_str, std::string search_key, std::string &version);
+amdsmi_status_t get_lspci_root_switch(amdsmi_bdf_t device_bdf, amdsmi_bdf_t *switchBdf);
 
-#endif //AMD_SMI_LSPCI_COMMANDS_H_
+#endif  // AMD_SMI_LSPCI_COMMANDS_H_

projects/amdsmi/include/amd_smi/impl/nic/amd_smi_no_drm_nic.h

@@ -52,14 +52,14 @@ class AMDSmiNoDrmNIC {
 
    uint32_t get_vendor_id();
     amdsmi_status_t amd_query_nic_info(uint32_t nic_index, amdsmi_brcm_nic_info_t& info);
-    amdsmi_status_t amd_query_nic_uuid(std::string devicePath, std::string& version);
-    amdsmi_status_t amd_query_nic_temp(std::string hwmonPath, amdsmi_brcm_nic_temperature_metric_t& info);
-    amdsmi_status_t amd_query_nic_device(std::string hwmonPath, amdsmi_brcm_nic_hwmon_device_t& info);
-    amdsmi_status_t amd_query_nic_power(std::string hwmonPath, amdsmi_brcm_nic_hwmon_power_t& info);
-    amdsmi_status_t amd_query_nic_numa_affinity(std::string devicePath, int32_t *numa_node);
-    amdsmi_status_t amd_query_nic_cpu_affinity(std::string devicePath, std::string& cpu_affinity);
+    amdsmi_status_t amd_query_nic_uuid(std::string device_path, std::string& version);
+    amdsmi_status_t amd_query_nic_temp(std::string hwmon_path, amdsmi_brcm_nic_temperature_metric_t& info);
+    amdsmi_status_t amd_query_nic_device(std::string hwmon_path, amdsmi_brcm_nic_hwmon_device_t& info);
+    amdsmi_status_t amd_query_nic_power(std::string hwmon_path, amdsmi_brcm_nic_hwmon_power_t& info);
+    amdsmi_status_t amd_query_nic_numa_affinity(std::string device_path, int32_t *numa_node);
+    amdsmi_status_t amd_query_nic_cpu_affinity(std::string device_path, std::string& cpu_affinity);
 
-    amdsmi_status_t amd_query_nic_fw_info(std::string devicePath, amdsmi_brcm_nic_firmware_t& info);
+    amdsmi_status_t amd_query_nic_fw_info(std::string device_path, amdsmi_brcm_nic_firmware_t& info);
  private:
     // when file is not found, the empty string will be returned
     std::vector<std::string> device_paths_;