Closed epall closed 6 years ago
Hey Eric,
Had a brief look at this pull request and if yep, if you could e.g. take the less contentious commits (more ether types, static info about pcap frames, framing exception) and do a new pull request for those and then we can continue on the "bigger" task at hand, which is the re-structuring of the packet hierarchy. And if I understand your logic behind it is that e.g. a packet of type X is a Packet and whatever it was transported across is somewhat irrelevant. Hence, by decoupling the assumption that Type X is a Type Y you can now actually use Type X across anything. If that is correct, then I can get behind that reasoning.
Let me know...
Great! Will do.
And if I understand your logic behind it is that e.g. a packet of type X is a Packet and whatever it was transported across is somewhat irrelevant. Hence, by decoupling the assumption that Type X is a Type Y you can now actually use Type X across anything. If that is correct, then I can get behind that reasoning.
Yes, exactly. It dramatically reduces coupling between the various layer implementations.
Packet hierarchy refactor submitted as #69
And I actually introduced FramingException in #64, so let's continue that conversation over there.
The only thing left is IPv6 support, which I'd like to put off writing a PR for until we get this Packet refactor figured out.
I'm working on a tool to analyze LAN traffic, and to do that I need to dissect a whole bunch of different packets. As such, I went ahead and gave IPv6 support a shot here. While I was at it, I added a few more EtherTypes and some aggressive validation of pcap frames. The validation is useful for my ingest tool, which breaks pcap files at arbitrary byte boundaries, so I have to seek forward to find a valid frame.
In the process of exploring IPv6 support, I wound up taking a crack at refactoring the Packet type hierarchy so that each type doesn't implement the interface of the enclosing packet. This makes more sense to me, but it might break existing usage. What do you think?
I'd rather break this up into several smaller changes, but first I want to get your opinion on which of these changes should land.