Skip to content
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.

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

Strip null bytes from parsed eeprom data before writing to db on S6000 platform #20512

Merged
merged 1 commit into from
Oct 23, 2024

Conversation

assrinivasan
Copy link
Contributor

@assrinivasan assrinivasan commented Oct 15, 2024

Why I did it

This was failing sonic-mgmt test test_syseepromd due to a setup error on account of null bytes being parsed from the S6000 eeprom.

This is due to differences in the way sonic-db-cli handles null values in the part number as parsed by the EEPROM in the 202012 image vs 202205+ image versions:

202305 image

admin@dell-s6000-device:~$ sudo sonic-installer list
Current: SONiC-OS-20230531.38
Next: SONiC-OS-20230531.38
Available:
SONiC-OS-20230531.38
admin@dell-s6000-device:~$
admin@dell-s6000-device:~$ redis-cli -n 6 HGETALL "EEPROM_INFO|Part Number"
1) "Value"
2) "00NX1V\x00\x00\x00\x00"
admin@dell-s6000-device:~$ sonic-db-cli STATE_DB  HGETALL "EEPROM_INFO|Part Number"
{'Value': '00NX1V'}
admin@dell-s6000-device:~$ sonic-db-cli STATE_DB  HGETALL "EEPROM_INFO|Part Number" | cat --show-nonprinting
{'Value': '00NX1V^@^@^@^@'}
admin@dell-s6000-device:~$

202012 image

admin@dell-s6000-device:~$ sudo sonic-installer list
Current: SONiC-OS-20201231.119
Next: SONiC-OS-20201231.119
Available:
SONiC-OS-20201231.119
SONiC-OS-20230531.38
admin@dell-s6000-device:~$
admin@dell-s6000-device:~$ redis-cli -n 6 HGETALL "EEPROM_INFO|Part Number"
1) "Value"
2) "00NX1V\x00\x00\x00\x00"
admin@dell-s6000-device:~$
admin@dell-s6000-device:~$ sonic-db-cli STATE_DB HGETALL "EEPROM_INFO|Part Number"
{'Value': '00NX1V\x00\x00\x00\x00'}
admin@dell-s6000-device:~$
admin@dell-s6000-device:~$ sonic-db-cli STATE_DB HGETALL "EEPROM_INFO|Part Number" | cat --show-nonprinting
{'Value': '00NX1V\x00\x00\x00\x00'}
admin@dell-s6000-device:~$

Work item tracking
  • Microsoft ADO (number only): 29720798

How I did it

Added logic to strip null bytes from parsed data before writing to database.

How to verify it

Run the aforementioned sonic-mgmt test before and after making the change in this PR -- note that test fails on setup without the change. See attached logs.
s6000_test_syseepromd_logs.txt

Flashed image with this change on an S6000 device and ran the above sonic-mgmt test:

Image info:

admin@sonic-device:~$ show ver

SONiC Software Version: SONiC.master-20512.669582-5884dfa4f
SONiC OS Version: 12
Distribution: Debian 12.6
Kernel: 6.1.0-22-2-amd64
Build commit: 5884dfa4f
Build date: Tue Oct 15 23:44:02 UTC 2024
Built by: redacted

Platform: x86_64-dell_s6000_s1220-r0
HwSKU: Force10-S6000
ASIC: broadcom
ASIC Count: 1
Serial Number: NA
Model Number: NA
Hardware Revision: NA
Uptime: 21:43:14 up 12 min,  1 user,  load average: 11.27, 10.11, 5.95
Date: Wed 16 Oct 2024 21:43:14

Relevant sonic-mgmt test logs:

------------------------------------------------------------------------------------------------------- live log call --------------------------------------------------------------------------------------------------------
21:47:19 sonic.get_pmon_daemon_status             L0914 INFO   | Daemon 'syseepromd' in the 'RUNNING' state with pid 36
21:47:19 test_syseepromd.test_pmon_syseepromd_sto L0139 INFO   | syseepromd daemon is RUNNING with pid 36
21:47:34 sonic.get_pmon_daemon_status             L0914 INFO   | Daemon 'syseepromd' in the 'STOPPED' state with pid -1
21:48:06 sonic.get_pmon_daemon_status             L0914 INFO   | Daemon 'syseepromd' in the 'RUNNING' state with pid 130
PASSED

Which release branch to backport (provide reason below if selected)

  • 202205
  • 202305
  • 202311

Tested branch (Please provide the tested image version)

  • [20220531.51 ]
  • [ 20230531.38]
  • [ 20230531.39]

Description for the changelog

Strip null bytes from parsed eeprom data before writing to db

Link to config_db schema for YANG module changes

A picture of a cute animal (not mandatory but encouraged)

@assrinivasan assrinivasan requested a review from lguohan as a code owner October 15, 2024 22:39
@assrinivasan assrinivasan changed the title Strip null bytes from parsed eeprom data before writing to db Strip null bytes from parsed eeprom data before writing to db in S6000 eeprom module Oct 15, 2024
@assrinivasan assrinivasan changed the title Strip null bytes from parsed eeprom data before writing to db in S6000 eeprom module Strip null bytes from parsed eeprom data before writing to db on S6000 platform Oct 15, 2024
@assrinivasan
Copy link
Contributor Author

Hi @santhosh-kt -- could you please help review this PR? I am unable to add you as a reviewer, for some reason. TIA.

@prgeor
Copy link
Contributor

prgeor commented Oct 18, 2024

@yxieca could you merge

@yxieca yxieca merged commit d4fe0bd into sonic-net:master Oct 23, 2024
12 checks passed
@assrinivasan
Copy link
Contributor Author

Hello @yxieca @StormLiangMS @bingwang-ms -- please help cherrypicking this PR to 202305, 202311 and 202405 branches. Thanks in advance.

mssonicbld pushed a commit to mssonicbld/sonic-buildimage that referenced this pull request Oct 25, 2024
…net#20512)

Why I did it
This was failing sonic-mgmt test test_syseepromd due to a setup error on account of null bytes being parsed from the S6000 eeprom.

This is due to differences in the way sonic-db-cli handles null values in the part number as parsed by the EEPROM in the 202012 image vs 202205+ image versions:

How I did it
Added logic to strip null bytes from parsed data before writing to database.

How to verify it
Run the aforementioned sonic-mgmt test before and after making the change in this PR -- note that test fails on setup without the change. See attached logs.
s6000_test_syseepromd_logs.txt

Flashed image with this change on an S6000 device and ran the above sonic-mgmt test:
@mssonicbld
Copy link
Collaborator

Cherry-pick PR to 202311: #20611

mssonicbld pushed a commit to mssonicbld/sonic-buildimage that referenced this pull request Oct 25, 2024
…net#20512)

Why I did it
This was failing sonic-mgmt test test_syseepromd due to a setup error on account of null bytes being parsed from the S6000 eeprom.

This is due to differences in the way sonic-db-cli handles null values in the part number as parsed by the EEPROM in the 202012 image vs 202205+ image versions:

How I did it
Added logic to strip null bytes from parsed data before writing to database.

How to verify it
Run the aforementioned sonic-mgmt test before and after making the change in this PR -- note that test fails on setup without the change. See attached logs.
s6000_test_syseepromd_logs.txt

Flashed image with this change on an S6000 device and ran the above sonic-mgmt test:
@mssonicbld
Copy link
Collaborator

Cherry-pick PR to 202405: #20612

mssonicbld pushed a commit that referenced this pull request Oct 25, 2024
Why I did it
This was failing sonic-mgmt test test_syseepromd due to a setup error on account of null bytes being parsed from the S6000 eeprom.

This is due to differences in the way sonic-db-cli handles null values in the part number as parsed by the EEPROM in the 202012 image vs 202205+ image versions:

How I did it
Added logic to strip null bytes from parsed data before writing to database.

How to verify it
Run the aforementioned sonic-mgmt test before and after making the change in this PR -- note that test fails on setup without the change. See attached logs.
s6000_test_syseepromd_logs.txt

Flashed image with this change on an S6000 device and ran the above sonic-mgmt test:
mssonicbld pushed a commit that referenced this pull request Oct 25, 2024
Why I did it
This was failing sonic-mgmt test test_syseepromd due to a setup error on account of null bytes being parsed from the S6000 eeprom.

This is due to differences in the way sonic-db-cli handles null values in the part number as parsed by the EEPROM in the 202012 image vs 202205+ image versions:

How I did it
Added logic to strip null bytes from parsed data before writing to database.

How to verify it
Run the aforementioned sonic-mgmt test before and after making the change in this PR -- note that test fails on setup without the change. See attached logs.
s6000_test_syseepromd_logs.txt

Flashed image with this change on an S6000 device and ran the above sonic-mgmt test:
rkavitha-hcl pushed a commit to rkavitha-hcl/sonic-buildimage that referenced this pull request Nov 15, 2024
…net#20512)

Why I did it
This was failing sonic-mgmt test test_syseepromd due to a setup error on account of null bytes being parsed from the S6000 eeprom.

This is due to differences in the way sonic-db-cli handles null values in the part number as parsed by the EEPROM in the 202012 image vs 202205+ image versions:

How I did it
Added logic to strip null bytes from parsed data before writing to database.

How to verify it
Run the aforementioned sonic-mgmt test before and after making the change in this PR -- note that test fails on setup without the change. See attached logs.
s6000_test_syseepromd_logs.txt

Flashed image with this change on an S6000 device and ran the above sonic-mgmt test:
aidan-gallagher pushed a commit to aidan-gallagher/sonic-buildimage that referenced this pull request Nov 16, 2024
…net#20512)

Why I did it
This was failing sonic-mgmt test test_syseepromd due to a setup error on account of null bytes being parsed from the S6000 eeprom.

This is due to differences in the way sonic-db-cli handles null values in the part number as parsed by the EEPROM in the 202012 image vs 202205+ image versions:

How I did it
Added logic to strip null bytes from parsed data before writing to database.

How to verify it
Run the aforementioned sonic-mgmt test before and after making the change in this PR -- note that test fails on setup without the change. See attached logs.
s6000_test_syseepromd_logs.txt

Flashed image with this change on an S6000 device and ran the above sonic-mgmt test:
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants