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

Added SLL2 layer decoding #1

Merged
merged 1 commit into from
Aug 16, 2022
Merged

Added SLL2 layer decoding #1

merged 1 commit into from
Aug 16, 2022

Conversation

wader
Copy link
Contributor

@wader wader commented Aug 14, 2022

google/gopacket#933 rebased, regenerated with linter and benchmark commits skipped as i'm not sure they are relevant anymore.

I did a similar PR google/gopacket#1015 sometime ago before i found the one by @dylandreimerink

Done rudimentary manual testing of pcaps. Should some kind of test(s) be added?

@mosajjal
Copy link
Contributor

just converted some of the travis checks into Github ones. gonna close and reopen this to trigger it :)

@mosajjal mosajjal closed this Aug 16, 2022
@mosajjal mosajjal reopened this Aug 16, 2022
@wader
Copy link
Contributor Author

wader commented Aug 16, 2022

@mosajjal 👍 no worries

@mosajjal
Copy link
Contributor

looks good and doesn't break any existing code. However, it doesn't look like it has any tests. If you have any tests ready to commit that'd be good.

@wader
Copy link
Contributor Author

wader commented Aug 16, 2022

There was no tests in the original PR but i can try add something. What you be a good test, parse some minimal sll2 pcap test file?

@mosajjal
Copy link
Contributor

take a look at layers tests like dns. For the sake of consistency we can dump the pcap into bytes for testing purposes. makes it easier imo

@wader
Copy link
Contributor Author

wader commented Aug 16, 2022

Added a test case but only included bytes from sll2 and down, looked like other tests only tested from their layer and down.

@mosajjal mosajjal merged commit 60750a3 into gopacket:master Aug 16, 2022
@mosajjal
Copy link
Contributor

Yep I gather it's the same. looks good to me :) merged

@wader
Copy link
Contributor Author

wader commented Aug 16, 2022

Confused, what is this about Go code is not formatted, run 'go fmt github.com/google/stenographer/...? should say go fmt ./...?

A did a fmt and lots of things changed... but i gess you have a PR for this?

@wader
Copy link
Contributor Author

wader commented Aug 16, 2022

@mosajjal Aha, good thanks!

@wader wader deleted the sll2 branch August 16, 2022 11:17
@mosajjal
Copy link
Contributor

yep that's an old relic from the Google's repo and Travis CI stuff. hopefully all will get removed in a couple days :)

@wader
Copy link
Contributor Author

wader commented Aug 16, 2022

👍 Great, and thanks for picking up the gopacket development again!

wader added a commit to wader/fq that referenced this pull request Aug 19, 2022
SLL2 support merged upstream gopacket/gopacket#1

This also cuts down a bit on some indirect deps
robbkidd pushed a commit to honeycombio/gopacket that referenced this pull request Sep 15, 2023
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.

None yet

2 participants