Skip to content
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

Discussion for changes in IpPacket #800

Closed
thvdveld opened this issue Jun 13, 2023 · 0 comments
Closed

Discussion for changes in IpPacket #800

thvdveld opened this issue Jun 13, 2023 · 0 comments

Comments

@thvdveld
Copy link
Contributor

IpPacket

At the moment, the IpPacket struct is very simple: it holds an IP header and a payload (ICMP, UDP, TCP, etc.). However, IP packets can be much more than that. For example, IPv6 can have a fragmentation header, routing header, hop-by-hop-header, etc. The current representation of the packet it too simple, making the processing/creating of IP packets difficult when working with routing headers and hop-by-hop headers. Since we are adding more features to smoltcp, we need to make IpPacket future-proof. In my RPL branch, I modified IpPacket such that it looks like the following (not complete and removed lifetimes):

enum IpPacket {
  Ipv4(Ipv4Packet),
  Ipv6(Ipv6Packet),
}

struct Ipv6Packet {
  header: Ipv6Repr,
  routing: Ipv6RoutingRepr,
  hbh: Option<Ipv6HopByHopRepr>,
  payload: Ipv6Payload,
}

enum Ipv6Payload {
  Icmpv6(Icmpv6Repr),
  Udp(UdpRepr, &[u8]),
  Tcp(TcpRepr),
  Raw(&[u8]),
}

Of course, now the IpPacket is way bigger than it was before. And it will grow if we add more headers and stuff.

Options data in Repr

Another change that I made in my RPL branch is that the options in the representations are not a slice to the option data, but a heapless Vec containing parsed options. The reason is the following: we first process IP packets. These functions return an Option<IpPacket>, which contains the IP packet that needs to get dispatched. In my opinion, this packet should already be complete.

When processing a RPL solicitation packet, I need to respond with a RPL DIO packet. This packet contains option data, and thus having a slice in the Repr is just not possible. I cannot create the data in the processing function, and then return a slice to that emitted data. I could add this data in the dispatching functions, however, this just is weird and error-prone.

Changing the slice in the Repr into a heapless::Vec makes ICMPv6Repr not Copy anymore (since RPL is part of ICMPv6), which is not ideal.

Goal of this issue

What I want in this issue is a discussion on how we can change IpPacket, such that it is easier to work with when we are adding more features. Is it OK if we change IpPacket like I describe it? And what about optional data in Repr's, which are created when processing incoming packets?

The changes I made here are incorporated in my RPL branch: https://github.com/thvdveld/smoltcp/tree/smoltcp-rpl-all-mops. I'm currently making small PR's to add RPL into smoltcp and want to make better design decisions than in my RPL branch, if possible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

1 participant