-
Notifications
You must be signed in to change notification settings - Fork 485
Add new socket type for raw ethernet protocols #1013
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?
Conversation
7b7b67f
to
682e851
Compare
682e851
to
de3eaae
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1013 +/- ##
==========================================
+ Coverage 80.45% 80.66% +0.20%
==========================================
Files 81 82 +1
Lines 24461 24829 +368
==========================================
+ Hits 19681 20028 +347
- Misses 4780 4801 +21 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
de3eaae
to
ef11c4f
Compare
I missed this PR and made similar thing, so... +1 for raw ethernet sockets |
ef11c4f
to
1146477
Compare
Something you might want to add the Right now I'm getting the compile error: compile_error!("If you enable the socket feature, you must enable at least one of the following features: socket-raw, socket-udp, socket-tcp, socket-icmp, socket-dhcpv4, socket-dns"); |
fb9eae3
to
833d319
Compare
It's cool to hear that others would like to use this lib in similar ways as we do. But I'm not sure if it's really worth investing more time into polishing this PR. |
@ususdei sorry it took so long before the first reply. I think this PR looks good. Could you, as @not-Ryan suggested, edit the compile error as well? I don't have any other remarks. @Dirbaio @whitequark any remarks? I think this feature was requested already in the issues (#942, #902 could be implemented as a |
@ususdei Apologies for the lack of feedback. I don't see any reason not to merge this; it fits the overall theme of the library and doesn't add any overhead when disabled. The implementation is basically copied from raw sockets, right? That's also an easy thing to approve. |
Since your force-push of 833d319 it seems to be broken at: ususdei@833d319#diff-57320a1d1f958a93a7d782cab2e9e6409dd32f50a98a86e854e88b28559de4b0R62-R65 This is only when you disable the default features:
|
Thanks @not-Ryan ! |
833d319
to
194bb69
Compare
I just finished some remaining TODOs from my list and reran the code on my test-device. So from my side, this MR is now ready for a thorough review. |
Exactly. I didn't see a way to extend raw sockets to also handle plain ethernet frames without breaking the And then copied over metadata support from UDP sockets (which is still missing from raw sockets but I need the metadata-id for accurate timestamps). |
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.
Can you explain the changes to ARP logic you've made?
} | ||
} | ||
|
||
/// A Eth packet metadata. |
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.
/// A Eth packet metadata. | |
/// An Ethernet packet metadata. |
/// A Eth packet metadata. | ||
pub type PacketMetadata = crate::storage::PacketMetadata<EthMetadata>; | ||
|
||
/// A Eth packet ring buffer. |
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.
/// A Eth packet ring buffer. | |
/// An Ethernet packet ring buffer. |
Hi everyone!
I'm currently working on a rust library for a protocol which runs on plain ethernet using a custom ethertype (TECMP on 0x99FE).
It also uses an additional IP-based protocol for discovery and configuration.
I would also like to use this library on some STM32, so I tried to port it from native linux sockets to the smoltcp stack.
Thus I started to implement some basic support for raw ethernet sockets.
This is still a work in progress but the basic functionality already works on an STM32F7.
It is still missing support for Metadata and the send API could be more convenient
but I would like to get this out there and gather some early feedback as to whether you would even consider merging something like this.
So just some initial questions:
Thank you all for your feedback and your great contribution to the embedded rust ecosystem!