[broadcom] Add broadcom ASIC info to "show version" click command #3621
Conversation
==== What I did
When the ASIC platform is broadcom, expand the "show version" output
with additional versions. This helps keeping track of different
versions when building/testing.
==== How I did it
Added _get_broadcom_info() function that calls 'bcmcmd' with 'bsv' and
'show unit'.
==== How to verify it
Run "show version" on a device with broadcom ASIC and observe how it now
lists this:
# show version
SONiC Software Version: SONiC.master.0-123456789
SONiC OS Version: 12
Distribution: Debian 12.8
Kernel: 6.1.0-22-2-amd64
Build commit: 123456789
Build date: Tue Nov 12 15:28:30 UTC 2024
Built by: sonic-builder
Platform: x86_64-accton_as9716_32d-r0
HwSKU: Accton-AS9716-32D
ASIC: broadcom
ASIC Count: 1
+ASIC API BRCM SAI ver: [10.1.42.0]
+ASIC API OCP SAI ver: [1.13.2]
+ASIC API SDK ver: [sdk-6.5.29]
+ASIC Model: Unit 0 chip BCM56980_B0 (current)
Serial Number: ...
On other platforms, that info will not be added. On any regular failure
(command not found, permission denied, command rejected, unexpected
output), show version will still complete.
wdoekes
force-pushed
the
show-version-add-broadcom-info
branch
from
8ef76b9 to
6d90f4d
Compare
| @@ -1496,6 +1508,8 @@ def version(verbose): | |||
| click.echo("HwSKU: {}".format(platform_info['hwsku'])) | |||
| click.echo("ASIC: {}".format(platform_info['asic_type'])) | |||
| click.echo("ASIC Count: {}".format(platform_info['asic_count'])) | |||
| if asic_info: | |||
There was a problem hiding this comment.
would this be disruptive for some test cases if adding output in the middle of show version cli?
There was a problem hiding this comment.
Legit question.
If I look at tests/show_platform_test.py, it should not change (for now) because it has a mellanox ASIC_TYPE and not a broadcom one.
My reason for placing it in the middle was this: the top info can then be copy-pasted into bug reports without including machine-specific serial numbers.
If you want, I can add a test there for the broadcom model with this output included.
There was a problem hiding this comment.
B.t.w. I'd rather fetch the info once at initialization from the ASIC and get it from a state(?) db during show version, but that's probably more invasive.
There was a problem hiding this comment.
my thought is better to keep output backward compatibility, since we are not sure if there's other customize scripts would be broken.
There was a problem hiding this comment.
Ok ok.
So, after the second paragraph and before the docker image list then?
What I did
When the ASIC platform is broadcom, expand the "show version" output with additional versions. This helps keeping track of different versions when building/testing.
How I did it
Added _get_broadcom_info() function that calls 'bcmcmd' with 'bsv' and 'show unit'.
How to verify it
Run
show versionon a device with broadcom ASIC and observe how it now lists this:Previous command output (if the output of a command-line utility has changed)
(see diff above)
New command output (if the output of a command-line utility has changed)
(see diff above)
Additional comments
I am aware that:
show versioncommand takes slightly longer because of the timeouts. I think this is acceptable.In my case, I could not get
masterfrom November 2024 to run on the Accton-AS7326-56X or Accton-AS9716-32D, except when I reverted ffb9bc021 and 433d039a4; reverting the BRCM SAI/SDK versions. While the BRCM/SAI versions will not get fetched if syncd is down (they will be in syslog), these clues are helpful when comparing output.Another output: