-
Notifications
You must be signed in to change notification settings - Fork 20
Basic networking support #76
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
base: main
Are you sure you want to change the base?
Basic networking support #76
Conversation
05f37da
to
08a3c69
Compare
This is super neat, though I wonder how useful blocking network APIs to Rust when it seems like much of the protocols that might work on this (written in Rust...) would likely want async/await? Are there any crates that would work with this API near term? E.g. crates I imagine I might want to try with Zephyr, Rust, and Networking https://crates.io/crates/picoserve Does the network API maybe need some small changes to make it work with these crates and make it async friendly? |
Any idea how many of these would even work with nostd? |
So, I do think that embassy-net would be useful to model after, but it isn't built around trait, just concrete implementations, so directly working with crates depending on that isn't going to be that easy. Long term would be to come up with HAL or at least traits to define a networking interface. I haven't looked too deeply into it, but given the noalloc focus, I suspect the API of embassy-net is quite different than would would naturally fit with Zephyr. As far as async, the usability depends on what exactly is provided by us. It looks like the socket interface is built around poll. Which means that we're going to need to add support to the executor (or alongside it, not sure what is workable). If anything, we'll need something like possibly a single task that can invoke the poll and call the right waker. I don't see any evidence of a callback-based interface, which is generally the best interface to use with async (it just calls the waker). Given Zephyr's move away from callbacks, due to userspace, we're probably going to find a growing semantic mismatch between Zephyr's interfaces and Rust async. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall, I think this is a pretty good start. The sockaddr stuff is just going to take some careful review, as it is unpleasant, and difficult to get right even in C. But, mostly it is just about unions (without names).
zephyr/src/error.rs
Outdated
/// signatures) return -1 on error, with the specific error code stored in the `errno` variable. | ||
#[inline(always)] | ||
pub fn to_result_errno<T: PartialOrd + Zero>(code: T) -> Result<T> { | ||
if code < T::zero() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would num::Signed::is_negative()
be clearer here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I hadn't spotted the Signed
trait - that does make more sense, since at the moment you could pass it an unsigned value which doesn't make much sense.
zephyr/src/net/ipaddr.rs
Outdated
pub(crate) fn ipv4_addr_to_c(addr: &Ipv4Addr) -> in_addr { | ||
let octets = addr.octets(); | ||
|
||
// It's not possible to easily construct this type due to the BindgenUnion field, and in any |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There will need to be a comment above each unsafe
block that starts with // SAFETY:
to describe the safety. This one is probably fine, just needs the comment.
|
||
/// docs todo... | ||
pub(crate) fn ipv4_addr_from_c(addr: &in_addr) -> Ipv4Addr { | ||
let s4_addr = unsafe { addr.__bindgen_anon_1.s4_addr.as_ref() }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Unsafe comment assumed)
Presumably the names __bindgen_anon_1
etc are going to be stable in bindgen, I guess based on the ordering of the declaration.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the closest to an answer I can find: https://docs.rs/bindgen/latest/bindgen/struct.Builder.html#method.anon_fields_prefix
So it seems that if we don't explicitly set the anon_fields_prefix
then __bindgen_anon_
followed by an integer identifier is used. It doesn't explicitly say how the integer identifier is generated, but since we only have one anonymous field in each of these structs it's probably safe to assume it will always be 1 (and in any case would generate an error at compile time if this is not the case)
The two crates listed both work with no_std, both are built on async/await. Any of the blocking network protocol crates are likely going to require std and all the heavy baggage that implies around memory allocation. |
Agreed that the blocking network APIs are not that useful for enabling existing networking crates from the Rust ecosystem to be used on Zephyr. But since the blocking API's are relatively easy to implement on top of the I do have some ideas for async support in the longer term, I was thinking that this could be provided as a separate set of structs built on top of the
I was imagining the implementation could use either a thread calling As far as usability and working with existing crates goes, the traits from embedded-nal-async and edge-nal may be worth a look. |
08a3c69
to
320779c
Compare
Added some documentation, and SAFETY comments for the unsafe blocks (excluding unsafe blocks that are only required for FFI calls - can document these too if needed but they seem to be skipped in the rest of the repo too at the moment). Also added TCP support, again loosely based on what's available in the |
This will need to be rebased, as the |
Generate Rust bindings for network socket and IP address symbols Signed-off-by: Matt Rodgers <[email protected]>
Various Zephyr functions have a return code where '-1' indicates an error, but the exact error code is stored in the errno variable. Add a helper function to convert this kind of return value into a Result. Signed-off-by: Matt Rodgers <[email protected]>
Initial implementation of a Rust wrapper around the Zephyr networking API for UDP sockets, loosely based on the UdpSocket in the Rust std library. It's possible to create/bind a socket and sendto/recvfrom data, but not much else. There is also some supporting functionality for converting IP addresses and socket addresses between the C representation and Rust's core::net types. Signed-off-by: Matt Rodgers <[email protected]>
Add an echo server sample to test UDP networking functionality.
Add TcpListener and TcpStream structs which work much like those in the Rust std library. Also add a basic test case to confirm that the TCP abstractions have the basic functions working. Signed-off-by: Matt Rodgers <[email protected]>
Spawn the UDP echo server in a thread, and run a TCP echo server in the main thread. Signed-off-by: Matt Rodgers <[email protected]>
320779c
to
1498212
Compare
Rebased, echo server sample works on native sim now which is a nice bonus! |
BTW, another change coming in will making it quite a bit easier to have async code happen based on a Zephyr semaphore. I'm not quite sure how to do this with the Zephyr network stack, which abstracts this quite a bit, though. But, overall, I think getting this in would be a helpful start. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is a good start.
Anything else needed for this to be merged? |
Initial networking support for blocking TCP and UDP sockets.
The API is based roughly on the
UdpSocket
,TcpListener
andTcpStream
types from the Rust standard library.For non-async networking, there don't appear to be any widely used traits that would be useful to implement - after having a closer look at
embedded_nal
, it seems that even the non-async version expects non-blocking behaviour, with read functions just returningnb::Error::WouldBlock
if no data is ready. This kind of makes sense, as I'm not aware of any other embedded implementation of a blocking network stack in Rust.The implementation is broadly split into:
socket.rs
: low level wrappers around thezsock_socket
functions.ipaddr.rs
: supporting functionality for converting IP and socket addresses between the C representation and the Rustcore::net
equivalents.mod.rs
: UDP and TCP functionalityThere are also some basic test cases for UDP and TCP, and a sample "echo server" application.
I've tested the echo server on
qemu_cortex_m3
(via the QEMU Ethernet interface) and on real hardware with thenucleo_h753zi
board.I think there's probably some other functionality needed for this to be really useful, in particular:
embedded_io_async
traits would enable the use of higher level networking libraries from the embedded Rust ecosystemBut to avoid the PR getting too large, I think this initial functionality is probably ready for review?