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

Removing 'rawNvConfig' to refrain of unstable NIC states in DPU mode #52

Merged
merged 1 commit into from
Nov 19, 2024

Conversation

heyvister1
Copy link
Contributor

@heyvister1 heyvister1 commented Nov 19, 2024

Removing 'rawNvConfig' since it was causing BF3/2 NICs to enter an unstable state once changing NIC mode from NIC to DPU

We should add and fix unstable states on next releases

@heyvister1 heyvister1 force-pushed the remove-raw-nv-config branch 2 times, most recently from b1deb06 to f3ed293 Compare November 19, 2024 11:56
@coveralls
Copy link

coveralls commented Nov 19, 2024

Pull Request Test Coverage Report for Build 11915792861

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.06%) to 58.866%

Totals Coverage Status
Change from base Build 11815892262: 0.06%
Covered Lines: 1786
Relevant Lines: 3034

💛 - Coveralls

@@ -200,15 +199,6 @@ func (v *configValidationImpl) ConstructNvParamMapFromTemplate(
applyDefaultNvConfigValueIfExists(consts.AtsEnabledParam, desiredParameters, query)
}

for _, rawParam := range template.RawNvConfig {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you comment out these lines and put a TODO to uncomment them once a fix is ready?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Name: "TEST_P2",
Value: "test",
},
},
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done. Although we can just check this PR changes :-)

@heyvister1 heyvister1 requested a review from e0ne November 19, 2024 14:30
…stable state.

We should add and fix unstable states on next releases
@e0ne e0ne merged commit 3932e77 into Mellanox:main Nov 19, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants