smoltcp-rs / smoltcp

a smol tcp/ip stack
BSD Zero Clause License
3.64k stars 404 forks source link

Parse full IPv6 packet before processing #832

Closed thvdveld closed 9 months ago

thvdveld commented 10 months ago

With these changes, an IPv6 packet is fully parsed before being processed by other functions. I made these changes, because for RPL, IPv6 hop-by-hop headers and routing headers need to be modified after they are parsed such that packets can be forwarded/processed correctly. For example, if an hop-by-hop is present followed by a routing header, first the hop-by-hop needs to be processed before the routing header. With the implementation currently in main, it is not clear if the processing in of the headers is done in this order. It is also not easy to keep these headers around until they need to be forwarded.

With the changes, an IPv6 packet is parsed into an Ipv6Packet (not the one from wire). This packet contains a parsed IPv6 header, Hop-by-Hop header, Routing header, Fragmentation header and a payload, which may be TCP, UDP, ICMPv6, etc. With the changes, when a packet needs to be forwarded, the parsed IPv6 packet can now easily be modified and returned by the process_ipv6 function, such that the packet is forwarded with the correct headers.

At first, I thought that the modifications I made will impact the binary sizes and make it way bigger, however, I was surprised to see that it's even a tiny bit smaller now.

Example server (before):

   text   data     bss     dec     hex  filename
1482212  81272     536  1564020 17dd74  server

Example server (after):

   text   data     bss     dec     hex  filename
1482060  80800     536  1563396 17db04  server

Example sixlowpan (before):

   text   data     bss     dec     hex  filename
1469581  80256     536  1550373 17a825  sixlowpan

Example sixlowpan (after):

   text   data     bss     dec     hex  filename
1469429  79784     536  1549749 17a5b5  sixlowpan
codecov[bot] commented 10 months ago

Codecov Report

Merging #832 (91d3d7d) into main (c790174) will increase coverage by 0.02%. Report is 23 commits behind head on main. The diff coverage is 70.62%.

@@            Coverage Diff             @@
##             main     #832      +/-   ##
==========================================
+ Coverage   79.39%   79.42%   +0.02%     
==========================================
  Files          76       78       +2     
  Lines       27504    27795     +291     
==========================================
+ Hits        21836    22075     +239     
- Misses       5668     5720      +52     
Files Changed Coverage Δ
src/iface/interface/sixlowpan.rs 87.61% <0.00%> (ø)
src/iface/ip_packet.rs 46.61% <0.00%> (-0.45%) :arrow_down:
src/iface/interface/mod.rs 45.81% <53.33%> (+0.18%) :arrow_up:
src/iface/interface/ipv4.rs 67.11% <57.14%> (-1.39%) :arrow_down:
src/iface/interface/ipv6.rs 79.26% <75.00%> (-12.12%) :arrow_down:
src/wire/ipv6hbh.rs 100.00% <100.00%> (ø)
src/wire/ipv6routing.rs 81.61% <100.00%> (ø)

... and 4 files with indirect coverage changes

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

thvdveld commented 10 months ago

I think that with these changes, we can remove a buffer for 6LoWPAN, where the decompression of incoming packets create an Ipv6Packet representation (not from wire), instead of writing bytes to a buffer and then process the bytes as IPv6.

Edit: still need to find a workaround for raw sockets, because then we need the raw data instead of the parsed data.

thvdveld commented 9 months ago

Therefore, extension headers must be processed strictly in the order they appear in the packet.

This PR breaks this, since we can't assume that the order is the same as the recommended order. Also, when the checksum of an UDP packet would be wrong, that check would fail and thus it would drop the packet, even though the packet might need to be forwarded instead.

I'm therefore closing this PR.