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

Fix PacketCapture pcapng file issue on macOS #6804

Merged
merged 1 commit into from
Nov 20, 2024

Conversation

hangyan
Copy link
Member

@hangyan hangyan commented Nov 12, 2024

By default, gopacket will write snap length=0 in the pcapng file header, means unlimited snaplen. tcpdump on osx(libpcap version 1.10.1) cannot recognize this and will report error. This patch will set a default value(524288) for it. This patch also add packets file verification in e2e tests.

origin error message:

tcpdump: pcap_loop: invalid packet capture length 74, bigger than snaplen of 524288

pcapng format ref: https://pcapng.com/

@hangyan hangyan requested a review from tnqn November 12, 2024 04:21
@luolanzone luolanzone added kind/bug Categorizes issue or PR as related to a bug. action/backport Indicates a PR that requires backports. labels Nov 12, 2024
@hangyan hangyan force-pushed the fix-packetcapture-on-mac branch 2 times, most recently from 01870b6 to 12e1973 Compare November 12, 2024 09:49
@antoninbas antoninbas changed the title Fix PacketCpature packets unrecognized issue on osx. Fix PacketCapture pcapng file issue on macOS Nov 12, 2024
pkg/agent/packetcapture/packetcapture_controller.go Outdated Show resolved Hide resolved
pkg/agent/packetcapture/packetcapture_controller.go Outdated Show resolved Hide resolved
test/e2e/packetcapture_test.go Outdated Show resolved Hide resolved
test/e2e/packetcapture_test.go Outdated Show resolved Hide resolved
test/e2e/packetcapture_test.go Outdated Show resolved Hide resolved
test/e2e/packetcapture_test.go Outdated Show resolved Hide resolved
@luolanzone luolanzone added the action/release-note Indicates a PR that should be included in release notes. label Nov 13, 2024
test/e2e/packetcapture_test.go Outdated Show resolved Hide resolved
test/e2e/packetcapture_test.go Outdated Show resolved Hide resolved
test/e2e/packetcapture_test.go Outdated Show resolved Hide resolved
@hangyan hangyan force-pushed the fix-packetcapture-on-mac branch from e011320 to 32682e8 Compare November 14, 2024 02:22
antoninbas
antoninbas previously approved these changes Nov 15, 2024
Copy link
Contributor

@antoninbas antoninbas left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for making the changes

@tnqn do you want to take a look?

test/e2e/packetcapture_test.go Outdated Show resolved Hide resolved
test/e2e/packetcapture_test.go Outdated Show resolved Hide resolved
Copy link
Member

@tnqn tnqn left a comment

Choose a reason for hiding this comment

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

LGTM

By default, gopacket will write snap length=0 in the pcapng file
header, means unlimited snaplen. tcpdump on osx(libpcap version 1.10.1)
cannot recognize this and will report error. This patch will set
a default value(65535) for it.

Signed-off-by: Hang Yan <[email protected]>
@hangyan hangyan force-pushed the fix-packetcapture-on-mac branch from d44f261 to cdb9e83 Compare November 20, 2024 09:24
@antoninbas
Copy link
Contributor

/test-all

@antoninbas
Copy link
Contributor

Unrelated e2e test failures in TestAntreaPolicy

@antoninbas antoninbas merged commit 5d2d4f3 into antrea-io:main Nov 20, 2024
55 of 56 checks passed
@luolanzone
Copy link
Contributor

@hangyan can you back port this PR to release 2.2.

@hangyan
Copy link
Member Author

hangyan commented Nov 21, 2024

@hangyan can you back port this PR to release 2.2.

sure

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
action/backport Indicates a PR that requires backports. action/release-note Indicates a PR that should be included in release notes. kind/bug Categorizes issue or PR as related to a bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants