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

[dhcpv4] Fix buffer end location #447

Merged
merged 2 commits into from
Oct 25, 2021
Merged

[dhcpv4] Fix buffer end location #447

merged 2 commits into from
Oct 25, 2021

Conversation

noiz
Copy link
Contributor

@noiz noiz commented Oct 21, 2021

Fix payload buffer to end at actual payload end. UDP header length includes the 8 bytes of the header so we do not want the +8 at the end of the buffer.

In our testing on ARM platforms we were hitting the return ErrInvalidOptions error when parsing DHCPv4 options at the end of the payload. It turns out the error was here that the payload buffer was 8 bytes too long so sometimes there would be garbage bytes after the optEnd byte that would cause this error to get returned. With this change we are now seeing the payload buffer size in the Golang code matches the payload buffer size we see in wireshark.

@noiz noiz changed the title Fix buffer end location [dhcpv4] Fix buffer end location Oct 21, 2021
@pmazzini
Copy link
Collaborator

Please sign your commit, example:

git commit --amend --no-edit --signoff
git push --force-with-lease origin master

Fix payload buffer to end at actual payload end. UDP header length includes the 8 bytes of the header so we do not want the +8 at the end of the buffer.

Signed-off-by: noiz <noiz@users.noreply.github.com>
@codecov
Copy link

codecov bot commented Oct 25, 2021

Codecov Report

Merging #447 (3c6a668) into master (63954f9) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #447   +/-   ##
=======================================
  Coverage   67.97%   67.97%           
=======================================
  Files          90       90           
  Lines        3766     3766           
=======================================
  Hits         2560     2560           
  Misses       1037     1037           
  Partials      169      169           
Flag Coverage Δ
integtests ∅ <ø> (∅)
unittests 67.97% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.


Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 63954f9...3c6a668. Read the comment docs.

Copy link
Collaborator

@pmazzini pmazzini left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems legit, thanks

@pmazzini pmazzini merged commit b259fbf into insomniacslk:master Oct 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants