Skip to content

Implementation of IPv4 Extended Echo Request/Response #883

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

nickludwig
Copy link

As specified in RFC 8337, an implementation of the IPv4 Extended Echo Request and Response within the print-icmp.c file. The included .pcap file is titled icmp-ext-ipv4.pcap

Link to RFC: https://tools.ietf.org/html/rfc8335#section-2.1
Link to Wireshark implementation, where I found and used the example ipv4 pcap file: https://gitlab.com/wireshark/wireshark/-/issues/14457

@infrastation
Copy link
Member

Thank you for suggesting this improvement. This pull request has 9 "work in progress" commits, is this the right branch?

@infrastation
Copy link
Member

Obviously, the CI builds did not pass because of the compiler warnings, please run touch .devel && ./configure && make clean && make -s all (item 4 in CONTRIBUTING) to see them locally and to fix them.

@nickludwig
Copy link
Author

nickludwig commented Oct 13, 2020 via email

@nickludwig
Copy link
Author

Fixed the previous issues.

@infrastation
Copy link
Member

Thank you. Please do not include configure in this case even if it is slightly different, also please find a good place in tests/TESTLIST and follow items 6 and 7 of CONTRIBUTING as closely as possible to amend the proposed commit.

@nickludwig nickludwig force-pushed the nick branch 2 times, most recently from 155d6d5 to 68d8f3a Compare October 30, 2020 00:18
@fxlb
Copy link
Member

fxlb commented Oct 31, 2020

configure file removed => some builds broken.

@nickludwig
Copy link
Author

Thanks! Should be fixed now.

@nickludwig
Copy link
Author

@infrastation, I followed all of the CONTRIBUTING guidelines and think it's ready to go. Please let me know if there is anything else that I can help with.

@fxlb
Copy link
Member

fxlb commented May 19, 2021

As specified in RFC 8337, an implementation of the IPv4 Extended [...]
(also in the commit message)

Or RFC 8335?

@fxlb
Copy link
Member

fxlb commented Oct 17, 2022

First look, prior to further review.

  1. Need a rebase on top of [the-tcpdump-group:master] (not a merge, to keep a readable history).
  2. 'switch (icmp_code)' could be replaced by a 'struct tok' and a tok2str() call.
  3. 'switch (reply_state & STATE_MASK)': same.
  4. Fix typo: uses of RFC 8337 instead of RFC 8335 (Conversation and commit message)
    (Implementation of IPv4 Extended Echo Request/Response #883 (comment))
  5. Remove trailing spaces and tabs.
  6. Check [ ] "Allow edits from maintainers" may help.

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

Successfully merging this pull request may close these issues.

3 participants