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

NetworkPkg/NvmeOfDxe: Resolve NBFT population issues #35

Conversation

trevor-cockrell
Copy link

@trevor-cockrell trevor-cockrell commented Apr 4, 2024

  • Adjust HFI population logic to no longer skip by MAC, instead check for a unique transport descriptor hash
  • Discovery ctlr population logic no longer treats each DeviceAdapter as an individual Discovery ctlr; Discovery ctlr entries now populated based on order in which unique Disc. ctlrs were found
  • Properly map SSNS, Discovery ctlrs to their respective primary HFI
  • Properly map SSNS records to their respective primary Discovery ctlr

Fixes #33

- Adjust HFI population logic to no longer skip by MAC, instead check
  for a unique transport descriptor hash
- Discovery ctlr population logic no longer treats each
  DeviceAdapter as an individual Discovery ctlr; Discovery ctlr entries
now populated based on order in which unique Disc. ctlrs were found
- Properly map SSNS, Discovery ctlrs to their respective primary HFI
- Properly map SSNS records to their respective primary Discovery ctlr

Signed-off-by: Trevor cockrell <trevor_cockrell@dell.com>
@trevor-cockrell
Copy link
Author

trevor-cockrell commented Apr 4, 2024

Addresses #34 , solution should also replace #33.

@tbzatek
Copy link

tbzatek commented Apr 9, 2024

Retested with clean efivars, I think I might have messed up my env. last time.

Fixed:

  • 33 - all SSNS -> HFI indexes are correct
  • 34 base scenario - both Discovery Descriptors present, correct SSNS -> DD indexes
  • 34 the second bad scenario - the second boot attempt pointing to an invalid IP address
    • both Discovery Descriptors present, the last SSNS record points to the second HFI and the second Discovery Descriptor

Still an issue:

  • 34 first bad scenario - the first boot attempt pointing to an invalid IP address
    • only one Discovery Descriptor and the HFI record for the (good) second boot attempt present
    • no extra SSNS record like in the opposite scenario, i.e. the first boot attempt is not reflected in the NBFT table in any way (no HFI, no Discovery, no SSNS corresponding records)
    • NBFT table attached: NBFT.gz

Copy link

@mwilck mwilck left a comment

Choose a reason for hiding this comment

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

Looks good to me. I didn't have time to do actual testing yet.

@trevor-cockrell
Copy link
Author

Still an issue: 34 first bad scenario - the first boot attempt pointing to an invalid IP address

@tbzatek I'm having trouble reproducing this issue on this branch with a clean build.
If I configure Attempt 1 with an invalid address and Attempt 2 with a valid target config, I still see HFI + SSNS + Discovery entries for the failed attempt.
Can you share more on your attempt configuration -- are the target addresses the only different items between the two?

@tbzatek
Copy link

tbzatek commented Jun 26, 2024

So the only thing I've changed in this scenario was the target IP address in Attempt 1. There's no gateway and the IP address is simply unreachable on the network.

scr1
scr2
scr3
scr4
scr5

This results in nvme_nbft_show.json

Looks like there's a difference between "interface link down" and "interface up but unreachable target" scenarios.

@johnmeneghini
Copy link

johnmeneghini commented Aug 29, 2024

Let's open a new issue to track the unfixed issues and merge this.

Still an issue:

34 first bad scenario - the first boot attempt pointing to an invalid IP address
only one Discovery Descriptor and the HFI record for the (good) second boot attempt present
no extra SSNS record like in the opposite scenario, i.e. the first boot attempt is not reflected in the NBFT table in any way (no HFI, no Discovery, no SSNS corresponding records)
NBFT table attached: NBFT.gz

@rahlvers rahlvers merged commit 6018114 into timberland_upstream-dev-full Aug 29, 2024
@rahlvers rahlvers deleted the timberland_upstream-dev-full_nbft-population-fixes branch August 29, 2024 16:16
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.

5 participants