Skip to content

SNP Integration Test: Improve Clarity #1621

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

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

SNP Integration Test: Improve Clarity #1621

wants to merge 6 commits into from

Conversation

phip1611
Copy link
Member

@phip1611 phip1611 commented Apr 11, 2025

TL;DR: This adds more clarity to facilitate future debugging and improvements for #1617.

Wow, this was an unexpected major time sink. The problem here is that if snp::test runs before pxe::test, snp.receive() receives an Ethernet ARP package rather than a Ethernet IPv4 packet.

I tried two solutions:

  • perform DHCP in SNP via PXEBaseCode protocol
  • Send a raw ethernet frame

I got non working, unfortunately. So instead, this adds more clarity to ease future debugging and improvements.

Relates to #1617 opened by @kraxel.

Checklist

  • Sensible git history (for example, squash "typo" or "fix" commits). See the Rewriting History guide for help.
  • Update the changelog (if necessary)

@phip1611 phip1611 self-assigned this Apr 11, 2025
@phip1611 phip1611 marked this pull request as draft April 11, 2025 07:24
@phip1611 phip1611 force-pushed the snp-test branch 2 times, most recently from ca3266b to e24e342 Compare April 15, 2025 10:33
@phip1611 phip1611 changed the title xxx fix snp test: make it idempotent and remove magic values SNP Integration Test: Improve Clarity Apr 15, 2025
@phip1611 phip1611 marked this pull request as ready for review April 15, 2025 10:39
@phip1611 phip1611 requested a review from nicholasbishop April 15, 2025 10:44
@phip1611 phip1611 force-pushed the snp-test branch 2 times, most recently from 6c758f5 to ff20426 Compare April 15, 2025 10:53
@kraxel
Copy link
Contributor

kraxel commented Apr 15, 2025

FYI: I wouldn't count on interrupt status returning anything useful. I'm actually somewhat surprised to find this in the SNP spec (without much details what the expected behavior is). UEFI does not use interrupts (except timer) and handles devices in polling mode. So there is little reason for uefi drivers to configure device interrupt registers in the first place.

@phip1611 phip1611 force-pushed the snp-test branch 4 times, most recently from 56f24c9 to ad4b06f Compare April 19, 2025 09:41
Interestingly, the interrupt status never shows that we
can receive a packet. Buggy OVMF firmware?
This helps to identify the network interface in integration tests.
So far, we also only use a single network device. In case there are
more, the next device should have a similar MAC address, such as
the last component being incremented by one.
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.

2 participants