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

refactor: move IPv6 HBH in own module #827

Merged
merged 1 commit into from
Aug 8, 2023

Conversation

thvdveld
Copy link
Contributor

@thvdveld thvdveld commented Aug 7, 2023

The Ipv6HopByHopRepr was previously the same as an Ipv6ExtHeader. However, the RFC says that hop-by-hop options might update en route, which is not possible with the current implementation.

I added a representation for the Hop-by-Hop header, which has a heapless Vec of parsed options. This way, we can modify the options when needed.

The function of the Ipv6ExtHeader struct is now purely for parsing the ext header type and the length. It also returns a pointer to the data it holds, which must be parsed by the correct extension header representations.

The Ipv6HopByHopRepr was previously the same as an Ipv6ExtHeader.
However, the RFC says that hop-by-hop options might change, which is not
possible with the current implementation.

I added a representatation for the Hop-by-Hop header, which has an
heapless Vec of parsed options. This way, we can modify the options when
needed.

The function of the Ipv6ExtHeader struct is now purely for parsing the
ext header type and the length. It also returns a pointer to the data it
holds, which must be parsed by the correct extension header
representations.
@codecov
Copy link

codecov bot commented Aug 7, 2023

Codecov Report

Merging #827 (53bdf43) into main (fa7fd3c) will increase coverage by 0.14%.
Report is 5 commits behind head on main.
The diff coverage is 97.22%.

@@            Coverage Diff             @@
##             main     #827      +/-   ##
==========================================
+ Coverage   79.65%   79.80%   +0.14%     
==========================================
  Files          72       72              
  Lines       27862    27991     +129     
==========================================
+ Hits        22193    22337     +144     
+ Misses       5669     5654      -15     
Files Changed Coverage Δ
src/wire/mod.rs 71.96% <ø> (ø)
src/wire/ipv6hbh.rs 97.02% <97.02%> (ø)
src/iface/interface/ipv6.rs 91.38% <100.00%> (ø)

... and 3 files with indirect coverage changes

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

@whitequark
Copy link
Contributor

Hm, why are we using both heapless and managed?

@whitequark
Copy link
Contributor

It looks like heapless was added in da1a2b2, which is fair enough (I didn't investigate why the DNS server needs it; I assume it does) but in the core of the stack I'd rather only see managed.

@thvdveld
Copy link
Contributor Author

thvdveld commented Aug 7, 2023

I think that'll be hard to pass the managed buffer to the parser no? The parse function should get a managed buffer from the interface, so then the interface inner should have a managed buffer, so then we need to implement a function that gives a managed buffer to the interface, which we then have to check if it was given by the user or not, and so on. I think it's simpler using the heapless::Vec in the wire module. I agree that using managed buffers for fragmentation and socket buffers is a must.

@whitequark
Copy link
Contributor

Yeah, on second thought you're right, go ahead.

@thvdveld thvdveld added this pull request to the merge queue Aug 8, 2023
Merged via the queue into smoltcp-rs:main with commit d3c491f Aug 8, 2023
14 checks passed
@thvdveld thvdveld deleted the ipv6-hop-by-hop branch August 8, 2023 07:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants