Skip to content

move to use max instead of if else in length, use list comprehension to simplify for loops #95

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
galrock wants to merge 6 commits into
base: master
Choose a base branch
from
Open

move to use max instead of if else in length, use list comprehension to simplify for loops #95

Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
c618db1
some changes
galrock Apr 3, 2023
5f626c6
move to class variables and change cust_args
galrock Apr 3, 2023
524e3ac
change width to max, decrease cyclomatic complexity
galrock Apr 3, 2023
bb4a5a4
revert class variable changes, remove __add__ for now until used, rem…
galrock Apr 4, 2023
8f1dd60
forgot to bring back a line and revert the while loop
galrock Apr 4, 2023
edfbb26
forgot the break
galrock Apr 4, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
153 changes: 73 additions & 80 deletions lshca/lshca.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,58 +37,59 @@


class Config(object):
log_level = "" # set by argparse

output_view = "system"
output_order_general = {
"system": ["Dev", "Desc", "PN", "PSID", "SN", "FW", "Driver", "PCI_addr", "RDMA", "Net", "Port", "Numa", "LnkStat",
"IpStat", "Link", "Rate", "SRIOV", "Parent_addr", "Tempr", "LnkCapWidth", "LnkStaWidth",
"HCA_Type", "Bond", "BondState", "BondMiiStat"],
"ib": ["Dev", "Desc", "PN", "PSID", "SN", "FW", "Driver", "RDMA", "Port", "Net", "Numa", "LnkStat", "IpStat",
"VrtHCA", "PLid", "PGuid", "IbNetPref"],
"roce": ["Dev", "Desc", "PN", "PSID", "SN", "FW", "Driver", "PCI_addr", "RDMA", "Net", "Port", "Numa", "LnkStat",
"IpStat", "RoCEstat"],
"cable": ["Dev", "Desc", "PN", "PSID", "SN", "FW", "Driver", "RDMA", "Net", "MST_device", "CblPN", "CblSN", "CblLng",
"PhyLinkStat", "PhyLnkSpd", "PhyAnalisys"],
"traffic": ["Dev", "Desc", "PN", "PSID", "SN", "FW", "Driver", "RDMA", "Net", "TX_bps", "RX_bps", "PktSeqErr"],
"lldp": ["Dev", "Desc", "PN", "PSID", "SN", "FW", "Driver", "PCI_addr", "RDMA", "Net", "Port", "Numa", "LnkStat",
"IpStat", "LLDPportId", "LLDPsysName", "LLDPmgmtAddr", "LLDPsysDescr"],
"dpu": ["Dev", "Desc", "PN", "PSID", "SN", "FW", "Driver", "PCI_addr", "RDMA", "Port", "Net", "DPUmode",
"BFBver", "RshimDev", "OvsBrdg", "LnkStat", "IpStat", "UplnkRepr", "PfRepr", "VfRepr", "SRIOV"]
}
output_order = output_order_general[output_view]
show_warnings_and_errors = True
colour_warnings_and_errors = True
warning_sign = "*"
error_sign = " >!<"
na_str = 'N/A'
na_str_extnd = '={}='.format(na_str)
in_use_by_vm_str = 'inUseByVM'

record_data_for_debug = False
record_dir = "/tmp/lshca"
record_tar_file = None

ver = "3.9"

output_format = "human_readable"
output_format_elastic = None
output_separator_char = "-"
output_fields_filter_positive = ""
output_fields_filter_negative = ""
where_output_filter = ""

# based on https://community.mellanox.com/s/article/lossless-roce-configuration-for-linux-drivers-in-dscp-based-qos-mode
lossless_roce_expected_trust = "dscp"
lossless_roce_expected_pfc = "00010000"
lossless_roce_expected_gtclass = "Global tclass=106"
lossless_roce_expected_tcp_ecn = "1"
lossless_roce_expected_rdma_cm_tos = "106"

# based on https://docs.mellanox.com/pages/viewpage.action?pageId=43714202#LinkLayerDiscoveryProtocol(LLDP)-lldptimer
lldp_capture_timeout = 35 # seconds. Based on default 30s value in Mellanox Onyx OS

@MrBr-github MrBr-github Apr 4, 2023

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I disagree with this section, please remove

def __init__(self):
# type: () -> None
self.log_level = "" # set by argparse

self.output_view = "system"
self.output_order_general = {
"system": ["Dev", "Desc", "PN", "PSID", "SN", "FW", "Driver", "PCI_addr", "RDMA", "Net", "Port", "Numa", "LnkStat",
"IpStat", "Link", "Rate", "SRIOV", "Parent_addr", "Tempr", "LnkCapWidth", "LnkStaWidth",
"HCA_Type", "Bond", "BondState", "BondMiiStat"],
"ib": ["Dev", "Desc", "PN", "PSID", "SN", "FW", "Driver", "RDMA", "Port", "Net", "Numa", "LnkStat", "IpStat",
"VrtHCA", "PLid", "PGuid", "IbNetPref"],
"roce": ["Dev", "Desc", "PN", "PSID", "SN", "FW", "Driver", "PCI_addr", "RDMA", "Net", "Port", "Numa", "LnkStat",
"IpStat", "RoCEstat"],
"cable": ["Dev", "Desc", "PN", "PSID", "SN", "FW", "Driver", "RDMA", "Net", "MST_device", "CblPN", "CblSN", "CblLng",
"PhyLinkStat", "PhyLnkSpd", "PhyAnalisys"],
"traffic": ["Dev", "Desc", "PN", "PSID", "SN", "FW", "Driver", "RDMA", "Net", "TX_bps", "RX_bps", "PktSeqErr"],
"lldp": ["Dev", "Desc", "PN", "PSID", "SN", "FW", "Driver", "PCI_addr", "RDMA", "Net", "Port", "Numa", "LnkStat",
"IpStat", "LLDPportId", "LLDPsysName", "LLDPmgmtAddr", "LLDPsysDescr"],
"dpu": ["Dev", "Desc", "PN", "PSID", "SN", "FW", "Driver", "PCI_addr", "RDMA", "Port", "Net", "DPUmode",
"BFBver", "RshimDev", "OvsBrdg", "LnkStat", "IpStat", "UplnkRepr", "PfRepr", "VfRepr", "SRIOV"]
}
self.output_order = self.output_order_general[self.output_view]
self.show_warnings_and_errors = True
self.colour_warnings_and_errors = True
self.warning_sign = "*"
self.error_sign = " >!<"
self.na_str = 'N/A'
self.na_str_extnd = '={}='.format(self.na_str)
self.in_use_by_vm_str = 'inUseByVM'

self.record_data_for_debug = False
self.record_dir = "/tmp/lshca"
self.record_tar_file = None

self.ver = "3.9"

self.output_format = "human_readable"
self.output_format_elastic = None
self.output_separator_char = "-"
self.output_fields_filter_positive = ""
self.output_fields_filter_negative = ""
self.where_output_filter = ""

# based on https://community.mellanox.com/s/article/lossless-roce-configuration-for-linux-drivers-in-dscp-based-qos-mode
self.lossless_roce_expected_trust = "dscp"
self.lossless_roce_expected_pfc = "00010000"
self.lossless_roce_expected_gtclass = "Global tclass=106"
self.lossless_roce_expected_tcp_ecn = "1"
self.lossless_roce_expected_rdma_cm_tos = "106"

# based on https://docs.mellanox.com/pages/viewpage.action?pageId=43714202#LinkLayerDiscoveryProtocol(LLDP)-lldptimer
self.lldp_capture_timeout = 35 # seconds. Based on default 30s value in Mellanox Onyx OS
pass

def parse_arguments(self, user_args):
# type: (list) -> None
Expand Down Expand Up @@ -158,9 +159,7 @@ def parse_arguments(self, user_args):
# comes to handle comma separated list of choices
cust_user_args = []
for arg in user_args:
result = arg.split(",")
for member in result:
cust_user_args.append(member)
cust_user_args = [x for x in arg.split(",")]

args = parser.parse_args(cust_user_args)
self.process_arguments(args)
Expand Down Expand Up @@ -394,7 +393,7 @@ def get_data(self):
for bdf in mlnx_bdf_list:
port_count = 1

while True:
while True and port_count < len(bdf_dev.port_list):

@MrBr-github MrBr-github Apr 4, 2023

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The introduced change has different functionality from existing code

bdf_dev = MlnxBDFDevice(bdf, self._data_source, self._config, port_count)
bdf_dev.get_data()
mlnx_bdf_devices.append(bdf_dev)
Expand All @@ -404,10 +403,6 @@ def get_data(self):
sf_dev.get_data()
mlnx_bdf_devices.append(sf_dev)


if port_count >= len(bdf_dev.port_list):
break

port_count += 1

# First handle all PFs
Expand Down Expand Up @@ -500,12 +495,15 @@ def __init__(self, config, data_source):

def append(self, data):
self.output.append(data)

def __add__(self,data):
self.output.append(data)

@MrBr-github MrBr-github Apr 4, 2023

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need it here?


def apply_select_output_filters(self):
# type: () -> None
if len(self.config.output_fields_filter_positive) > 0:
if self.config.output_fields_filter_positive:
self.output_order = self.config.output_fields_filter_positive
elif len(self.config.output_fields_filter_negative) > 0:
elif self.config.output_fields_filter_negative:
decrement_list = self.output_order

output_filter = self.config.output_fields_filter_negative
Expand All @@ -516,7 +514,7 @@ def apply_select_output_filters(self):
self.output_order = decrement_list

data_keys_remove_list = []
if len(self.output) > 0:
if self.output:
output_data_keys = list(self.output[0]) + list(self.output[0]["bdf_devices"][0])
data_keys_remove_list = list(set(output_data_keys) - set(self.output_order))

Expand Down Expand Up @@ -694,26 +692,23 @@ def update_separator_and_column_width(self):
# first pass: collect all of the maximum widths for each of the BDF fields
hca_field_line_width = 0
for hca in self.output:
for key in hca:
if key == "bdf_devices":
for bdf_device in hca["bdf_devices"]:
for bdf_key in bdf_device:
if bdf_key in self.output_order:
# decide what is longer the key name or it's value
if len(bdf_device[bdf_key]) > len(bdf_key):
width = len(bdf_device[bdf_key])
else:
width = len(bdf_key)
bdf_devices = [ hca[x] for x in hca if x=="bdf_devices"]
for bdf_device in bdf_devices:
for bdf_key in bdf_device:
if bdf_key in self.output_order:
# decide what is longer the key name or it's value
width = max([len(bdf_device[bdf_key]),len(bdf_key)])

if bdf_key not in self.column_width:
self.column_width[bdf_key] = width
if bdf_key not in self.column_width:
self.column_width[bdf_key] = width

if width > self.column_width[bdf_key]:
self.column_width[bdf_key] = width
if width > self.column_width[bdf_key]:
self.column_width[bdf_key] = width

# second pass: calculate width of BDF and HCA lines
for hca in self.output:
curr_hca_column_width = {}
bdf_devices = [hca[key] for key in hca if key == "bdf_devices"]
for key in hca:
if key == "bdf_devices":
for bdf_device in hca["bdf_devices"]:
Expand Down Expand Up @@ -779,11 +774,9 @@ def print_hca_header(self, args):
# type: (dict) -> None
order_dict = {}

position = 0
for key in self.output_order:
for idx, key in enumerate(self.output_order):
if key in args:
order_dict[key] = position
position += 1
order_dict[key] = idx

output_list = [""] * len(order_dict)
for key in args:
Expand Down