Skip to content

Commit

Permalink
Fix the sfputil treats page number as decimal instead of hexadecimal (s…
Browse files Browse the repository at this point in the history 
…onic-net#3153)

* Fix the sfputil treats page number as decimal instead of hexadecimal (sonic-net#22)

Fix the sfputil treats page number as decimal instead of hexadecimal

Signed-off-by: Kebo Liu <[email protected]>

* remove unreachable code

Signed-off-by: Kebo Liu <[email protected]>

---------

Signed-off-by: Kebo Liu <[email protected]>
  • Loading branch information
@keboliu
keboliu authored Feb 7, 2024
1 parent 167f996 commit a3cf5c0
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 39 deletions.
4 changes: 2 additions & 2 deletions doc/Command-Reference.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13060,7 +13060,7 @@ Usage: sfputil read-eeprom [OPTIONS]
Options:
-p, --port <logical_port_name> Logical port name [required]
-n, --page <page> EEPROM page number [required]
-n, --page <page> EEPROM page number in hex [required]
-o, --offset <offset> EEPROM offset within the page [required]
-s, --size <size> Size of byte to be read [required]
--no-format Display non formatted data
Expand Down Expand Up @@ -13092,7 +13092,7 @@ Usage: sfputil write-eeprom [OPTIONS]
Options:
-p, --port <logical_port_name> Logical port name [required]
-n, --page <page> EEPROM page number [required]
-n, --page <page> EEPROM page number in hex [required]
-o, --offset <offset> EEPROM offset within the page [required]
-d, --data <data> Hex string EEPROM data [required]
--wire-addr TEXT Wire address of sff8472
Expand Down
56 changes: 19 additions & 37 deletions sfputil/main.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -697,16 +697,20 @@ def eeprom(port, dump_dom, namespace):
# 'eeprom-hexdump' subcommand
@show.command()
@click.option('-p', '--port', metavar='<port_name>', help="Display SFP EEPROM hexdump for port <port_name>")
@click.option('-n', '--page', metavar='<page_number>', type=click.IntRange(0, MAX_EEPROM_PAGE), help="Display SFP EEEPROM hexdump for <page_number_in_hex>")
@click.option('-n', '--page', metavar='<page_number>', help="Display SFP EEEPROM hexdump for <page_number_in_hex>")
def eeprom_hexdump(port, page):
"""Display EEPROM hexdump of SFP transceiver(s)"""
if port:
if page is None:
page = 0
return_code, output = eeprom_hexdump_single_port(port, page)
else:
page = validate_eeprom_page(page)
return_code, output = eeprom_hexdump_single_port(port, int(str(page), base=16))
click.echo(output)
sys.exit(return_code)
else:
if page is not None:
page = validate_eeprom_page(page)
logical_port_list = natsorted(platform_sfputil.logical)
lines = []
for logical_port_name in logical_port_list:
Expand All @@ -718,27 +722,24 @@ def eeprom_hexdump(port, page):
lines.append(output)
click.echo('\n'.join(lines))


def validate_eeprom_page(page):
"""
Validate input page module EEPROM
Args:
page: str page input by user
Returns:
int page
"""
try:
page = int(page)
page = int(str(page), base=16)
except ValueError:
click.echo('Please enter a numeric page number')
sys.exit(ERROR_NOT_IMPLEMENTED)
if page < 0 or page > 255:
click.echo(f'Invalid page {page}')
if page < 0 or page > MAX_EEPROM_PAGE:
click.echo(f'Error: Invalid page number {page}')
sys.exit(ERROR_INVALID_PAGE)
return page


def eeprom_hexdump_single_port(logical_port_name, page):
"""
Dump EEPROM for a single logical port in hex format.
Expand Down Expand Up @@ -830,7 +831,7 @@ def eeprom_hexdump_pages_general(logical_port_name, pages, target_page):
tuple(0, dump string) if success else tuple(error_code, error_message)
"""
if target_page is not None:
lines = [f'EEPROM hexdump for port {logical_port_name} page {target_page}h']
lines = [f'EEPROM hexdump for port {logical_port_name} page {target_page:x}h']
else:
lines = [f'EEPROM hexdump for port {logical_port_name}']
physical_port = logical_port_to_physical_port_index(logical_port_name)
Expand Down Expand Up @@ -871,7 +872,7 @@ def eeprom_hexdump_pages_sff8472(logical_port_name, pages, target_page):
tuple(0, dump string) if success else tuple(error_code, error_message)
"""
if target_page is not None:
lines = [f'EEPROM hexdump for port {logical_port_name} page {target_page}h']
lines = [f'EEPROM hexdump for port {logical_port_name} page {target_page:x}h']
else:
lines = [f'EEPROM hexdump for port {logical_port_name}']
physical_port = logical_port_to_physical_port_index(logical_port_name)
Expand Down Expand Up @@ -928,28 +929,6 @@ def eeprom_dump_general(physical_port, page, flat_offset, size, page_offset, no_
return 0, ''.join('{:02x}'.format(x) for x in page_dump)



def eeprom_dump_general(physical_port, page, flat_offset, size, page_offset, no_format=False):
"""
Dump module EEPROM for given pages in hex format.
Args:
logical_port_name: logical port name
pages: a list of pages to be dumped. The list always include a default page list and the target_page input by
user
target_page: user input page number, optional. target_page is only for display purpose
Returns:
tuple(0, dump string) if success else tuple(error_code, error_message)
"""
sfp = platform_chassis.get_sfp(physical_port)
page_dump = sfp.read_eeprom(flat_offset, size)
if page_dump is None:
return ERROR_NOT_IMPLEMENTED, f'Error: Failed to read EEPROM for page {page:x}h, flat_offset {flat_offset}, page_offset {page_offset}, size {size}!'
if not no_format:
return 0, hexdump(EEPROM_DUMP_INDENT, page_dump, page_offset, start_newline=False)
else:
return 0, ''.join('{:02x}'.format(x) for x in page_dump)


def convert_byte_to_valid_ascii_char(byte):
if byte < 32 or 126 < byte:
return '.'
Expand Down Expand Up @@ -1737,7 +1716,7 @@ def target(port_name, target):
# 'read-eeprom' subcommand
@cli.command()
@click.option('-p', '--port', metavar='<logical_port_name>', help="Logical port name", required=True)
@click.option('-n', '--page', metavar='<page>', type=click.IntRange(0, MAX_EEPROM_PAGE), help="EEPROM page number", required=True)
@click.option('-n', '--page', metavar='<page>', help="EEPROM page number in hex", required=True)
@click.option('-o', '--offset', metavar='<offset>', type=click.IntRange(0, MAX_EEPROM_OFFSET), help="EEPROM offset within the page", required=True)
@click.option('-s', '--size', metavar='<size>', type=click.IntRange(1, MAX_EEPROM_OFFSET + 1), help="Size of byte to be read", required=True)
@click.option('--no-format', is_flag=True, help="Display non formatted data")
Expand Down Expand Up @@ -1765,6 +1744,8 @@ def read_eeprom(port, page, offset, size, no_format, wire_addr):
api = sfp.get_xcvr_api()
if api is None:
click.echo('Error: SFP EEPROM not detected!')
if page is not None:
page = validate_eeprom_page(page)
if not isinstance(api, sff8472.Sff8472Api):
overall_offset = get_overall_offset_general(api, page, offset, size)
else:
Expand All @@ -1785,7 +1766,7 @@ def read_eeprom(port, page, offset, size, no_format, wire_addr):
# 'write-eeprom' subcommand
@cli.command()
@click.option('-p', '--port', metavar='<logical_port_name>', help="Logical port name", required=True)
@click.option('-n', '--page', metavar='<page>', type=click.IntRange(0, MAX_EEPROM_PAGE), help="EEPROM page number", required=True)
@click.option('-n', '--page', metavar='<page>', help="EEPROM page number in hex", required=True)
@click.option('-o', '--offset', metavar='<offset>', type=click.IntRange(0, MAX_EEPROM_OFFSET), help="EEPROM offset within the page", required=True)
@click.option('-d', '--data', metavar='<data>', help="Hex string EEPROM data", required=True)
@click.option('--wire-addr', help="Wire address of sff8472")
Expand Down Expand Up @@ -1819,7 +1800,8 @@ def write_eeprom(port, page, offset, data, wire_addr, verify):
if api is None:
click.echo('Error: SFP EEPROM not detected!')
sys.exit(EXIT_FAIL)

if page is not None:
page = validate_eeprom_page(page)
if not isinstance(api, sff8472.Sff8472Api):
overall_offset = get_overall_offset_general(api, page, offset, len(bytes))
else:
Expand Down Expand Up @@ -1855,11 +1837,11 @@ def get_overall_offset_general(api, page, offset, size):
"""
if api.is_flat_memory():
if page != 0:
raise ValueError(f'Invalid page number {page}, only page 0 is supported')
raise ValueError(f'Invalid page number {page:x}h, only page 0 is supported')

if page != 0:
if offset < MIN_OFFSET_FOR_NON_PAGE0:
raise ValueError(f'Invalid offset {offset} for page {page}, valid range: [128, 255]')
raise ValueError(f'Invalid offset {offset} for page {page:x}h, valid range: [80h, FFh]')

if size + offset - 1 > MAX_EEPROM_OFFSET:
raise ValueError(f'Invalid size {size}, valid range: [1, {255 - offset + 1}]')
Expand Down

0 comments on commit a3cf5c0

Please sign in to comment.