JulianSchmid / etherparse

A rust library for parsing ethernet & ethernet using protocols.
Apache License 2.0
286 stars 54 forks source link

Add ARP module #15

Open lachrimae opened 2 years ago

lachrimae commented 2 years ago

I'd like to parse ARP packets for Ethernet+IPv4. Might you be interested in merging this if I complete the implementation?

codecov[bot] commented 2 years ago

Codecov Report

Merging #15 (cab1b5b) into master (1d2be79) will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #15   +/-   ##
=======================================
  Coverage   99.69%   99.69%           
=======================================
  Files          45       45           
  Lines       16320    16320           
=======================================
  Hits        16271    16271           
  Misses         49       49           
Impacted Files Coverage Δ
src/link/mod.rs 100.00% <ø> (ø)

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 1d2be79...cab1b5b. Read the comment docs.

ravenexp commented 2 years ago

Hi! I'm interested in using etherparse for parsing and building ARP packets.

My current implementation uses Ethernet2Header and a hand-coded ARP parser/builder. This PR contains a higher quality implementation that I'd like to adopt.

I somewhat disagree on the implementation approach though: there is no such thing as "ARP header" according to RFC 826. ARP defines fixed-length Ethernet packets that have a very simple structure. Breaking it down further into "ARP header" and "ARP payload" seems artificial to me.

Should I try to submit an independent PR or are you rather interested in merging the current one?

lachrimae commented 1 year ago

@ravenexp I am pretty happy for you to submit your own version of the ARP module with the improvements you have in mind. I haven't heard back from the maintainer on this thread so I'm not sure what they think.