Skip to content

Issue 431 network snp #541

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

Closed
wants to merge 8 commits into from
Closed

Issue 431 network snp #541

wants to merge 8 commits into from

Conversation

d-sonuga
Copy link
Contributor

Added bare bones of the SNP support but I'm stumped on the test.

The frame is getting transmitted but the it's never received.

@nicholasbishop
Copy link
Member

I've looked a bit at the test and implementation, and I don't have a clear answer as to why you aren't receiving a packet. Here are some notes:

  • Some uses of Option in the implementation are incorrect. The UEFI Specification uses OPTIONAL as a shorthand for "you can pass in NULL for a pointer". I'm not sure why the UINTN parameters to EFI_SIMPLE_NETWORK.Initialize in the spec are marked as OPTIONAL, since they aren't pointers. (The spec often has silly ambiguities/mistakes like this, but they don't really accept bug reports.) Option can only be used in FFI for certain types, see https://doc.rust-lang.org/std/option/index.html#representation. For example the uses of Option<&T> are fine, but not Option<usize>.
  • A similar layout issue is mcast_filter: Option<*const [MacAddress]>. That should be mcast_filter: *const MacAddress. In addition to the Option issue, pointers to slices in Rust are wide pointers containing both an address and a length, which is different from the C ABI.
  • Just an FYI for defining bitflag fields, you can use the bitflags! macro.
  • Rather retrying the receive call in a loop, I think what you want is to call BootServices::wait_for_event using the wait_for_packet. However, when I tried that the event never signaled as ready, so there is another unknown problem.

A couple questions:

Is the test code here based on an example from somewhere? I tried to find an example of someone using the simple network protocol on stackoverflow/github/etc, and didn't turn up anything. Maybe I just missed something though. As I understand the code, you are transmitting to the broadcast address, then trying to receive a packet. I'm not sure if sending a packet to yourself is an allowed use or not. Of course, if this is based on an existing example that could help clear things up.

Another thought I have is that after spending some more time reading the relevant parts of the spec, it seems the SNP is quite low-level. There are some higher level protocols like EFI_TCP4_PROTOCOL, and even the even higher level EFI_HTTP_SERVICE_BINDING_PROTOCOL. I wonder if it might make more sense to try implementing one of those higher-level protocols for now? I imagine it would be easier to figure out testing for the higher-level protocols, and we might even learn something that helps us figure out why the SNP test isn't working.

Hope that helps, let me know if you have any questions :)

@d-sonuga
Copy link
Contributor Author

d-sonuga commented Nov 1, 2022

Thank you very much @nicholasbishop. That was very helpful.
The test is based on the previous PR for implementing SNP.
I understand what you mean and I will switch to implementing a higher level protocol

@d-sonuga d-sonuga closed this Nov 1, 2022
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