smoltcp
smoltcp copied to clipboard
Expose the packet parsing and crafting infrastructure as a separate crate
Rust ecosystem lacks a mature packet parsing and crafting library similar to libtins in C++, the best I found is the packet crate which lacks some basic functionality like ARP packets (if a good library existed, I guess this crate would probably use it).
This crate has a great in house implementation of such functionality, which is zero copy, handles corner cases, is well maintained, ... . Would you accept a PR to exposing it as a separate crate, e.g. smolpackets
? Currently most (all?) of that functionality is private, which is reasonable since it is not the intended job of the smoltcp
crate. The new crate can still live in this repository, and I would expect no stability guarantees on it (it can even be published with the version of the smoltcp
crate or in some auto release fashion) but it will still probably increase the maintenance cost a little.
This idea came up more than once (matrix room of smoltcp). So I was working on this as well! I have a branch that is kind of ready. I'll create a pull request this afternoon.
I discussed this a bit in the matrix room. The thing is that it would be difficult to maintain it. People might want to merge exotic protocols, however, these protocols might not fit the use case of smoltcp and there is not really someone that wants to commit to maintain it.
Then there is a second problem: smoltcp has a lot of feature flags. With these feature flags, we can keep the memory footprint low, since I think most of the people use it for embedded. If we have two crates, then smolwire could enable proto-ipv6
and proto-ipv4
, and smoltcp might only enable proto-ipv6
, which will cause compile errors.
What is your use case? Couldn't you just use smoltcp::wire
directly?
So it seems what I need is basically the smoltcp::wire
. I thought I need smoltcp::iface::ip_packet::IpPayload
and friends but it looks like those are thin internal wrappers over smoltcp::wire
.
I still think separate crate makes sense and #812 is in the right direction. About the feature flags problem, from what I see, the only problem would be feature gated enum variants which disappear in pattern matching. Since smoltcp
depends on smolpackets
it can enable the corresponding features of it so feature flags of smoltcp
will always be a subset of feature flags of smolpackets
, and since those enums are marked (or should be marked as) non exhaustive, there would be no compile time error since there should always be a _
pattern there. Even if someone hits some compile error due mismatching in feature flags of these crates, they can easily solve the problem by depending directly on smoltcp
and adding the missing feature flags.
The maintaining concern is valid, though you can always reject anything that you see undesirable for the smoltcp
goals, and exotic old protocols are stable so there wouldn't be that much of maintaining costs (exotic new protocols should be unconditionally rejected by this logic). I personally don't need any exotic protocol and I think smoltcp::wire
already satisfies my needs.
there would be no compile time error since there should always be a _ pattern there
there's not always a _
, for example here https://github.com/smoltcp-rs/smoltcp/blob/8180cbcc3786f16f2d5d07df4d7cd644770dfbca/smoltcp/src/iface/interface/mod.rs#L1211-L1216
forcing adding _
to everywhere is a bit ugly
Cargo documentation recommends that enabling a feature flag should be a semver compatible change and adding variant to an exhaustive enum is a breaking change so smoltcp is already committing cargo crime and cargo police might catch us :) (that is, in a theoretical scenario, some library can depend on smoltcp
with feature proto-ipv4
and breaks if the root crate enables the other feature) and the situation will be worse if we switch to the multi crate setup. I see three solutions:
- Make
IpAddress
and similar enums#[non_exhaustive]
: Ugly as you said, but makes the cargo police happy and is minimal change relative to the current version. - Make
IpAddress
variants always enable, or usecore::net::IpAddr
: Special care needed in this case to not create unnecessary bloat, but I believe it is possible. Enum variants on their own don't generate any assembly, functions of them do, and we should have some care to useIpv4Addr
when possible (IpAddr
is unconditionally 17 bytes butIpv4Addr
is 4 bytes). It also won't save us from become forcing to add_
match arms in some cases. - Keep things as is. Most users will use one of the
smoltcp
orsmolpackets
crates and the root crate always can fix such compile errors, so it wouldn't be end of the world.