Skip to content

Commit

Permalink
Improve error handling and resource cleanup for PacketCapture
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.

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

Signed-off-by: Quan Tian <[email protected]>
  • Loading branch information
tnqn committed Nov 10, 2024
1 parent d9a3305 commit 4b0e411
Show file tree
Hide file tree
Showing 7 changed files with 415 additions and 411 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 4b0e411

Please sign in to comment.