Skip to content

Commit

Permalink
Improve error handling and resource cleanup for PacketCapture (#6797)
Browse files Browse the repository at this point in the history
The patch fixes the following issues:

1. The local pcap file was not closed on error. The patch ensures the
   file is closed regardless of success or failure.
2. The pcap file was not updated to `status.filePath` and was not
   uploaded to file server if not all packets have been captured. The
   patch ensures it's updated to status and uploaded as long as any
   packet is captured.
3. The Complete condition was not set correctly in one case. The patch
   uses two errors to track capturing error and uploading error
   separately.
4. The unit tests could report false negative as they only check that
   unexpected conditions don't exist, while they should check expected
   conditions exist.
5. Change the local pcap path's permission to 700.
6. Avoid panic when handling delete events containing
   DeletedFinalStateUnknown objects.
7. Reduce the min and max retry intervals to reduce the possibility of
   timeout.
8. Make a deepcopy before updating object got from lister, otherwise data
   race may happen and index may be messed up.
9. Add a readiness probe to the sftp server used in e2e test to avoid
   test flake.

It also makes some style improvements and simplifies a few code.

Signed-off-by: Quan Tian <[email protected]>
  • Loading branch information
tnqn authored Nov 10, 2024
1 parent d9a3305 commit 3ec5fa7
Show file tree
Hide file tree
Showing 7 changed files with 436 additions and 425 deletions.
24 changes: 13 additions & 11 deletions pkg/agent/packetcapture/capture/pcap_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,27 +40,29 @@ func NewPcapCapture() (*pcapCapture, error) {
}

func (p *pcapCapture) Capture(ctx context.Context, device string, srcIP, dstIP net.IP, packet *crdv1alpha1.Packet) (chan gopacket.Packet, error) {
eth, err := pcapgo.NewEthernetHandle(device)
// Compile the BPF filter in advance to reduce the time window between starting the capture and applying the filter.
inst := compilePacketFilter(packet, srcIP, dstIP)
klog.V(5).InfoS("Generated bpf instructions for PacketCapture", "device", device, "srcIP", srcIP, "dstIP", dstIP, "packetSpec", packet, "bpf", inst)
rawInst, err := bpf.Assemble(inst)
if err != nil {
return nil, err
}

eth.SetPromiscuous(false)
eth.SetCaptureLength(maxSnapshotBytes)

inst := compilePacketFilter(packet, srcIP, dstIP)
klog.V(5).InfoS("Generated bpf instructions for Packetcapture", "device", device, "srcIP", srcIP, "dstIP", dstIP, "packetSpec", packet, "bpf", inst)
rawInst, err := bpf.Assemble(inst)
eth, err := pcapgo.NewEthernetHandle(device)
if err != nil {
return nil, err
}
err = eth.SetBPF(rawInst)
if err != nil {
if err = eth.SetPromiscuous(false); err != nil {
return nil, err
}
if err = eth.SetBPF(rawInst); err != nil {
return nil, err
}
if err = eth.SetCaptureLength(maxSnapshotBytes); err != nil {
return nil, err
}

packetSource := gopacket.NewPacketSource(eth, layers.LinkTypeEthernet)
packetSource.NoCopy = true
packetSource := gopacket.NewPacketSource(eth, layers.LinkTypeEthernet, gopacket.WithNoCopy(true))
return packetSource.PacketsCtx(ctx), nil

}
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
//go:build !linux
// +build !linux

// Copyright 2024 Antrea Authors.
//
// Licensed under the Apache License, Version 2.0 (the "License");
Expand Down Expand Up @@ -28,9 +31,9 @@ type pcapCapture struct {
}

func NewPcapCapture() (*pcapCapture, error) {
return nil, errors.New("PacketCapture is not implemented on Windows")
return nil, errors.New("PacketCapture is not implemented")
}

func (p *pcapCapture) Capture(ctx context.Context, device string, srcIP, dstIP net.IP, packet *crdv1alpha1.Packet) (chan gopacket.Packet, error) {
return nil, errors.New("PacketCapture is not implemented on Windows")
return nil, errors.New("PacketCapture is not implemented")
}
Loading

0 comments on commit 3ec5fa7

Please sign in to comment.