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

Fix asic identification #15297

Merged
merged 3 commits into from
Nov 13, 2024

Conversation

ccroy-arista
Copy link
Contributor

Description of PR

ASIC identification is (mostly) done by matching entire lines from the output of the lspci command. This is prone to failure over time, if the output lines change for whatever reason. A more robust approach is to match only those tokens which can
be considered reliable in identifying and distinguishing the ASICs, as opposed to matching whole lines of output. This change implements the token-based approach.

Additionally, this PR introduces the token matching for the TH4 and TH5 asics.

Summary:
Fixes # (issue)

Type of change

  • Bug fix
  • Testbed and Framework(new/improvement)
  • Test case(new/improvement)

Back port request

  • 202012
  • 202205
  • 202305
  • 202311
  • 202405

Approach

What is the motivation for this PR?

test_everflow_per_interface.py was failing due to misidentified asic (i.e. asic was "unknown"), causing the test to execute a different code path than intended.

How did you do it?

Updated the matching logic to compare on the pertinent information returned from the lspci command, rather than entire portions of the returned lines.

How did you verify/test it?

Ran the test_everflow_per_interface.py against TH2, TH4, and TH5-based Arista DUTs and confirmed that the tests passed.

Device ASIC identification is achieved by
whole line matches from the output of lspci,
which is excessive and subject to fail due to
unforeseeable changes in such output.

This change reduces the string matching to
specific unique differentiators in the output
from lspci, while also future-proofing against
similar changes in the lspci that could
foreseeably occur.
Add token matches for identifying the TH4 and TH5
ASICs from the output of lspci.
@mssonicbld
Copy link
Collaborator

The pre-commit check detected issues in the files touched by this pull request.
The pre-commit check is a mandatory check, please fix detected issues.

Detailed pre-commit check results:
trim trailing whitespace.................................................Passed
fix end of files.........................................................Passed
check yaml...........................................(no files to check)Skipped
check for added large files..............................................Passed
check python ast.........................................................Passed
flake8...................................................................Failed
- hook id: flake8
- exit code: 1

tests/common/devices/sonic.py:37:1: E302 expected 2 blank lines, found 1

flake8...............................................(no files to check)Skipped
check conditional mark sort..........................(no files to check)Skipped

To run the pre-commit checks locally, you can follow below steps:

  1. Ensure that default python is python3. In sonic-mgmt docker container, default python is python2. You can run
    the check by activating the python3 virtual environment in sonic-mgmt docker container or outside of sonic-mgmt
    docker container.
  2. Ensure that the pre-commit package is installed:
sudo pip install pre-commit
  1. Go to repository root folder
  2. Install the pre-commit hooks:
pre-commit install
  1. Use pre-commit to check staged file:
pre-commit
  1. Alternatively, you can check committed files using:
pre-commit run --from-ref <commit_id> --to-ref <commit_id>

Fix pre-commit error introduced within the prior
two commits.
Copy link
Collaborator

@StormLiangMS StormLiangMS left a comment

Choose a reason for hiding this comment

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

LGTM

@StormLiangMS StormLiangMS merged commit 8f3e42a into sonic-net:master Nov 13, 2024
15 checks passed
mssonicbld pushed a commit to mssonicbld/sonic-mgmt that referenced this pull request Nov 13, 2024
* sonic-mgmt: improve asic identification

Device ASIC identification is achieved by
whole line matches from the output of lspci,
which is excessive and subject to fail due to
unforeseeable changes in such output.

This change reduces the string matching to
specific unique differentiators in the output
from lspci, while also future-proofing against
similar changes in the lspci that could
foreseeably occur.

* sonic-mgmt: add th4/th5 asic identification

Add token matches for identifying the TH4 and TH5
ASICs from the output of lspci.

* sonic-mgmt: fix pre-commit issue

Fix pre-commit error introduced within the prior
two commits.
@mssonicbld
Copy link
Collaborator

Cherry-pick PR to 202405: #15522

mssonicbld pushed a commit to mssonicbld/sonic-mgmt that referenced this pull request Nov 13, 2024
* sonic-mgmt: improve asic identification

Device ASIC identification is achieved by
whole line matches from the output of lspci,
which is excessive and subject to fail due to
unforeseeable changes in such output.

This change reduces the string matching to
specific unique differentiators in the output
from lspci, while also future-proofing against
similar changes in the lspci that could
foreseeably occur.

* sonic-mgmt: add th4/th5 asic identification

Add token matches for identifying the TH4 and TH5
ASICs from the output of lspci.

* sonic-mgmt: fix pre-commit issue

Fix pre-commit error introduced within the prior
two commits.
@ccroy-arista ccroy-arista deleted the fix-asic-identification branch November 13, 2024 19:39
mssonicbld pushed a commit that referenced this pull request Nov 15, 2024
* sonic-mgmt: improve asic identification

Device ASIC identification is achieved by
whole line matches from the output of lspci,
which is excessive and subject to fail due to
unforeseeable changes in such output.

This change reduces the string matching to
specific unique differentiators in the output
from lspci, while also future-proofing against
similar changes in the lspci that could
foreseeably occur.

* sonic-mgmt: add th4/th5 asic identification

Add token matches for identifying the TH4 and TH5
ASICs from the output of lspci.

* sonic-mgmt: fix pre-commit issue

Fix pre-commit error introduced within the prior
two commits.
sreejithsreekumaran pushed a commit to sreejithsreekumaran/sonic-mgmt that referenced this pull request Nov 15, 2024
* sonic-mgmt: improve asic identification

Device ASIC identification is achieved by
whole line matches from the output of lspci,
which is excessive and subject to fail due to
unforeseeable changes in such output.

This change reduces the string matching to
specific unique differentiators in the output
from lspci, while also future-proofing against
similar changes in the lspci that could
foreseeably occur.

* sonic-mgmt: add th4/th5 asic identification

Add token matches for identifying the TH4 and TH5
ASICs from the output of lspci.

* sonic-mgmt: fix pre-commit issue

Fix pre-commit error introduced within the prior
two commits.
yutongzhang-microsoft pushed a commit to yutongzhang-microsoft/sonic-mgmt that referenced this pull request Nov 21, 2024
* sonic-mgmt: improve asic identification

Device ASIC identification is achieved by
whole line matches from the output of lspci,
which is excessive and subject to fail due to
unforeseeable changes in such output.

This change reduces the string matching to
specific unique differentiators in the output
from lspci, while also future-proofing against
similar changes in the lspci that could
foreseeably occur.

* sonic-mgmt: add th4/th5 asic identification

Add token matches for identifying the TH4 and TH5
ASICs from the output of lspci.

* sonic-mgmt: fix pre-commit issue

Fix pre-commit error introduced within the prior
two commits.
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.

3 participants