Skip to content

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

2 participants