-
Notifications
You must be signed in to change notification settings - Fork 26
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
Revert a commit in ipxe that breaks Mellanox NICs #116
Conversation
2be4124
to
93ab6b2
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #116 +/- ##
=======================================
Coverage 97.28% 97.28%
=======================================
Files 5 5
Lines 442 442
=======================================
Hits 430 430
Misses 8 8
Partials 4 4 ☔ View full report in Codecov by Sentry. |
Hey @nshalman , im not seeing commit |
https://github.com/nshalman/ipxe/tree/fix-mellanox / nshalman/ipxe@fbc3b4a Because of how github works, ipxe/ipxe@fbc3b4a also resolves. |
hmmm...im thoroughly confused here. That link ipxe/ipxe@fbc3b4a shows me this message: |
oh i think i see, maybe. This is just GitHub doing redirects for us. Ultimately, this is a commit from your fork. correct? |
Correct. |
Relates to: tinkerbell#115, ipxe/ipxe#1091 This would update iPXE to the latest master plus a revert of a commit identified to break Mellanox NICs. This is a temporary measure that might need to be repeated on future updates of iPXE until ipxe/ipxe#1091 has been fixed correctly. Signed-off-by: Nahum Shalman <[email protected]>
93ab6b2
to
04747d7
Compare
I could make it explicit that we're pulling from my fork temporarily, but the nice thing about doing it this way is that as soon as ipxe is bumped the workaround is no longer in place. We fail securely back to upstream ipxe rather than risking staying stuck pointing at a fork. The worst possible outcome of future updates is that issue #115 (which will still be open) reappears in ipxedust. |
Description
This would update iPXE to the latest master plus a revert of a commit identified to break Mellanox NICs.
This is a temporary measure that might need to be repeated on future updates of iPXE until ipxe/ipxe#1091 has been fixed correctly.
Why is this needed
Relates to: #115, ipxe/ipxe#1091
How Has This Been Tested?
I did repeated testing of iPXE binaries from my branches
mainline-ipxe
test-revert
to confirm that the issue exists and is addressed by the revert.
How are existing users impacted? What migration steps/scripts do we need?
This would unbreak users of certain Mellanox NICs (Definitely Mellanox CX4, possibly others too)
Checklist:
I have: