From 2768afa5070723c914b38834e177fd8d891ad430 Mon Sep 17 00:00:00 2001 From: Hang Yan Date: Wed, 23 Oct 2024 18:12:06 +0800 Subject: [PATCH] update Signed-off-by: Hang Yan --- .../packetcapture/packetcapture_controller.go | 67 ++++++++++++++----- .../packetcapture_controller_test.go | 17 ++++- test/e2e/packetcapture_test.go | 41 +++++++----- 3 files changed, 91 insertions(+), 34 deletions(-) diff --git a/pkg/agent/controller/packetcapture/packetcapture_controller.go b/pkg/agent/controller/packetcapture/packetcapture_controller.go index 93f4573dd43..18ea428d6d5 100644 --- a/pkg/agent/controller/packetcapture/packetcapture_controller.go +++ b/pkg/agent/controller/packetcapture/packetcapture_controller.go @@ -89,6 +89,16 @@ var ( defaultFS = afero.NewOsFs() ) +// go-pcap seems doesn't support filter with numeric protocol number yet. use this map +// to translate to string. +var protocolMap = map[int]string{ + 1: "icmp", + 6: "tcp", + 17: "udp", + 58: "icmp6", + 132: "sctp", +} + func getPacketDirectory() string { return filepath.Join(os.TempDir(), "antrea", "packetcapture", "packets") } @@ -224,10 +234,18 @@ func (c *Controller) processPacketCaptureItem() bool { } func (c *Controller) cleanupPacketCapture(pcName string) { - if err := defaultFS.Remove(nameToPath(pcName)); err == nil { - klog.V(2).InfoS("Deleted pcap file", "name", pcName, "path", nameToPath(pcName)) + path := nameToPath(pcName) + exist, err := afero.Exists(defaultFS, path) + if err != nil { + klog.ErrorS(err, "Failed to check if path exists", "path", path) + } + if !exist { + return + } + if err := defaultFS.Remove(path); err == nil { + klog.V(2).InfoS("Deleted pcap file", "name", pcName, "path", path) } else { - klog.ErrorS(err, "Failed to delete pcap file", "name", pcName, "path", nameToPath(pcName)) + klog.ErrorS(err, "Failed to delete pcap file", "name", pcName, "path", path) } } @@ -306,11 +324,34 @@ func (c *Controller) startPacketCapture(pc *crdv1alpha1.PacketCapture) error { // 'src 192.168.0.1 and dst 192.168.0.2 and src port 8080 and dst port 8081 and tcp and tcp[tcpflags] & (tcp-syn|tcp-ack) != 0' func genBPFFilterStr(matchPacket *binding.Packet, packetSpec *crdv1alpha1.Packet) string { exp := "" + protocol := packetSpec.Protocol + if protocol != nil { + if protocol.Type == intstr.Int { + if val, ok := protocolMap[protocol.IntValue()]; ok { + exp += val + } else { + // go-pcap didn't support proto number for now. + exp += "proto " + strconv.Itoa(protocol.IntValue()) + } + } else { + exp += strings.ToLower(protocol.String()) + } + } + if exp != "" { + if exp == "icmp" || exp == "icmp6" { + // go-pcap bug, see:https://github.com/packetcap/go-pcap/issues/59 + // cannot use `and` now + exp += " " + } else { + exp += " and " + } + + } if matchPacket.SourceIP != nil { - exp += "src " + matchPacket.SourceIP.String() + exp += "src host " + matchPacket.SourceIP.String() } if matchPacket.DestinationIP != nil { - exp += " and dst " + matchPacket.DestinationIP.String() + exp += " and dst host " + matchPacket.DestinationIP.String() } if matchPacket.SourcePort > 0 { exp += " and src port " + strconv.Itoa(int(matchPacket.SourcePort)) @@ -318,14 +359,7 @@ func genBPFFilterStr(matchPacket *binding.Packet, packetSpec *crdv1alpha1.Packet if matchPacket.DestinationPort > 0 { exp += " and dst port " + strconv.Itoa(int(matchPacket.DestinationPort)) } - protocol := packetSpec.Protocol - if protocol != nil { - if protocol.Type == intstr.Int { - exp += " and proto " + strconv.Itoa(protocol.IntValue()) - } else { - exp += " and " + strings.ToLower(protocol.String()) - } - } + tcp := packetSpec.TransportHeader.TCP if tcp != nil { if tcp.Flags != nil { @@ -367,8 +401,8 @@ func (c *Controller) performCapture(captureState *packetCaptureState, device str if err != nil { return fmt.Errorf("couldn't write packet: %w", err) } - klog.V(2).InfoS("capture packet", "name", captureState.name, "count", - captureState.numCapturedPackets, "packet", packet.String()) + klog.V(9).InfoS("capture packet", "name", captureState.name, "count", + captureState.numCapturedPackets, "len", ci.Length) reachTarget := captureState.numCapturedPackets == captureState.maxNumCapturedPackets // use rate limiter to reduce the times we need to update status. @@ -392,6 +426,7 @@ func (c *Controller) performCapture(captureState *packetCaptureState, device str return err } } + err = c.updatePacketCaptureStatus(pc, crdv1alpha1.PacketCaptureRunning, "", captureState.numCapturedPackets) if err != nil { return fmt.Errorf("failed to update the PacketCapture: %w", err) @@ -403,6 +438,7 @@ func (c *Controller) performCapture(captureState *packetCaptureState, device str if err != nil { return fmt.Errorf("get PacketCapture failed: %w", err) } + klog.InfoS("PacketCapture timeout", "name", pc.Name) return c.updatePacketCaptureStatus(pc, crdv1alpha1.PacketCaptureFailed, captureTimeoutReason, 0) } } @@ -625,6 +661,7 @@ func (c *Controller) setPacketsFilePathStatus(name string) error { // checkPacketCaptureStatus is only called for PacketCaptures in the Running phase func (c *Controller) checkPacketCaptureStatus(pc *crdv1alpha1.PacketCapture) error { if checkPacketCaptureSucceeded(pc) { + klog.V(4).InfoS("PacketCapture succeeded", "name", pc.Name) return c.updatePacketCaptureStatus(pc, crdv1alpha1.PacketCaptureSucceeded, "", 0) } diff --git a/pkg/agent/controller/packetcapture/packetcapture_controller_test.go b/pkg/agent/controller/packetcapture/packetcapture_controller_test.go index 9e5ecbc5139..595015c76a2 100644 --- a/pkg/agent/controller/packetcapture/packetcapture_controller_test.go +++ b/pkg/agent/controller/packetcapture/packetcapture_controller_test.go @@ -57,7 +57,7 @@ var ( icmp6Proto = intstr.FromInt32(58) icmpProto = intstr.FromString("ICMP") tcpProto = intstr.FromString("TCP") - udpProto = intstr.FromInt32(16) + udpProto = intstr.FromInt32(17) port80 int32 = 80 port81 int32 = 81 @@ -478,7 +478,7 @@ func TestGenBPFFilterString(t *testing.T) { SourcePort: 80, DestinationPort: 81, }, - expected: "src 192.168.0.1 and dst 192.168.0.2 and src port 80 and dst port 81 and tcp and tcp[13] & 16 != 0", + expected: "tcp and src host 192.168.0.1 and dst host 192.168.0.2 and src port 80 and dst port 81 and tcp[13] & 16 != 0", }, { name: "udp no port and numeric protocol", @@ -490,7 +490,18 @@ func TestGenBPFFilterString(t *testing.T) { SourceIP: net.ParseIP("192.168.0.1"), DestinationIP: net.ParseIP("192.168.0.2"), }, - expected: "src 192.168.0.1 and dst 192.168.0.2 and proto 16", + expected: "udp and src host 192.168.0.1 and dst host 192.168.0.2", + }, + {name: "icmp with src and dest", + packetSpec: &crdv1alpha1.Packet{ + IPFamily: v1.IPv4Protocol, + Protocol: &icmpProto, + }, + matchPacket: &binding.Packet{ + SourceIP: net.ParseIP("192.168.0.1"), + DestinationIP: net.ParseIP("192.168.0.2"), + }, + expected: "icmp src host 192.168.0.1 and dst host 192.168.0.2", }, } for _, pt := range tt { diff --git a/test/e2e/packetcapture_test.go b/test/e2e/packetcapture_test.go index 13e8b36c191..a7cf32d4353 100644 --- a/test/e2e/packetcapture_test.go +++ b/test/e2e/packetcapture_test.go @@ -48,6 +48,9 @@ var ( icmp6Proto = intstr.FromInt32(58) testServerPort int32 = 80 + + pcTimeoutReason = "PacketCapture timeout" + pcShortTimeout = uint16(10) ) type pcTestCase struct { @@ -181,6 +184,7 @@ func TestPacketCapture(t *testing.T) { t.Run("testPacketCapture", func(t *testing.T) { testPacketCapture(t, data) }) + } func testPacketCapture(t *testing.T, data *TestData) { @@ -208,14 +212,15 @@ func testPacketCapture(t *testing.T, data *TestData) { testcases := []pcTestCase{ { - name: "to-ipv4-ip", + name: "timeout-case", ipVersion: 4, srcPod: pcToolboxPodName, pc: &crdv1alpha1.PacketCapture{ ObjectMeta: metav1.ObjectMeta{ - Name: randName(fmt.Sprintf("%s-%s-to-%s-%s-", data.testNamespace, pcToolboxPodName, data.testNamespace, tcpServerPodName)), + Name: randName(fmt.Sprintf("%s-timeout-case-", data.testNamespace)), }, Spec: crdv1alpha1.PacketCaptureSpec{ + Timeout: &pcShortTimeout, Source: crdv1alpha1.Source{ Pod: &crdv1alpha1.PodReference{ Namespace: data.testNamespace, @@ -245,8 +250,9 @@ func testPacketCapture(t *testing.T, data *TestData) { }, }, - expectedPhase: crdv1alpha1.PacketCaptureSucceeded, - expectedNum: 5, + expectedPhase: crdv1alpha1.PacketCaptureFailed, + expectedReason: pcTimeoutReason, + expectedNum: 5, }, } t.Run("testPacketCapture", func(t *testing.T) { @@ -292,7 +298,7 @@ func testPacketCaptureBasic(t *testing.T, data *TestData) { srcPod: pcToolboxPodName, pc: &crdv1alpha1.PacketCapture{ ObjectMeta: metav1.ObjectMeta{ - Name: randName(fmt.Sprintf("%s-%s-to-%s-%s-", data.testNamespace, pcToolboxPodName, data.testNamespace, tcpServerPodName)), + Name: randName(fmt.Sprintf("%s-ipv4-tcp-", data.testNamespace)), }, Spec: crdv1alpha1.PacketCaptureSpec{ Source: crdv1alpha1.Source{ @@ -335,7 +341,7 @@ func testPacketCaptureBasic(t *testing.T, data *TestData) { srcPod: pcToolboxPodName, pc: &crdv1alpha1.PacketCapture{ ObjectMeta: metav1.ObjectMeta{ - Name: randName(fmt.Sprintf("%s-%s-to-%s-%s-", data.testNamespace, pcToolboxPodName, data.testNamespace, udpServerPodName)), + Name: randName(fmt.Sprintf("%s-ipv4-udp-", data.testNamespace)), }, Spec: crdv1alpha1.PacketCaptureSpec{ Source: crdv1alpha1.Source{ @@ -378,7 +384,7 @@ func testPacketCaptureBasic(t *testing.T, data *TestData) { srcPod: node1Pods[0], pc: &crdv1alpha1.PacketCapture{ ObjectMeta: metav1.ObjectMeta{ - Name: randName(fmt.Sprintf("%s-%s-to-%s-%s-", data.testNamespace, node1Pods[0], data.testNamespace, node1Pods[1])), + Name: randName(fmt.Sprintf("%s-ipv4-icmp-", data.testNamespace)), }, Spec: crdv1alpha1.PacketCaptureSpec{ Source: crdv1alpha1.Source{ @@ -416,7 +422,7 @@ func testPacketCaptureBasic(t *testing.T, data *TestData) { srcPod: node1Pods[0], pc: &crdv1alpha1.PacketCapture{ ObjectMeta: metav1.ObjectMeta{ - Name: randName(fmt.Sprintf("%s-%s-to-%s-%s-ipv6", data.testNamespace, node1Pods[0], data.testNamespace, node1Pods[1])), + Name: randName(fmt.Sprintf("%s-ipv6-icmp-", data.testNamespace)), }, Spec: crdv1alpha1.PacketCaptureSpec{ Source: crdv1alpha1.Source{ @@ -455,7 +461,7 @@ func testPacketCaptureBasic(t *testing.T, data *TestData) { srcPod: node1Pods[0], pc: &crdv1alpha1.PacketCapture{ ObjectMeta: metav1.ObjectMeta{ - Name: randName(fmt.Sprintf("%s-%s-to-%s-%s-", data.testNamespace, node1Pods[0], data.testNamespace, nonExistPodName)), + Name: randName(fmt.Sprintf("%s-non-exist-pod-", data.testNamespace)), }, Spec: crdv1alpha1.PacketCaptureSpec{ Source: crdv1alpha1.Source{ @@ -533,7 +539,7 @@ func runPacketCaptureTest(t *testing.T, data *TestData, tc pcTestCase) { } }() - if dstPodName != nonExistPodName { + if dstPodName != nonExistPodName && tc.expectedReason != pcTimeoutReason { srcPod := tc.srcPod if dstIP := tc.pc.Spec.Destination.IP; dstIP != nil { ip := net.ParseIP(*dstIP) @@ -556,13 +562,13 @@ func runPacketCaptureTest(t *testing.T, data *TestData, tc pcTestCase) { t.Logf("Ping(%d) '%s' -> '%v' failed: ERROR (%v)", protocol, srcPod, *dstPodIPs, err) } } else if protocol == protocolTCP { - for i := 1; i <= 50; i++ { + for i := 1; i <= 10; i++ { if err := data.runNetcatCommandFromTestPodWithProtocol(tc.srcPod, data.testNamespace, toolboxContainerName, server, serverPodPort, "tcp"); err != nil { t.Logf("Netcat(TCP) '%s' -> '%v' failed: ERROR (%v)", srcPod, server, err) } } } else if protocol == protocolUDP { - for i := 1; i <= 50; i++ { + for i := 1; i <= 10; i++ { if err := data.runNetcatCommandFromTestPodWithProtocol(tc.srcPod, data.testNamespace, toolboxContainerName, server, serverPodPort, "udp"); err != nil { t.Logf("Netcat(UDP) '%s' -> '%v' failed: ERROR (%v)", srcPod, server, err) } @@ -579,12 +585,15 @@ func runPacketCaptureTest(t *testing.T, data *TestData, tc pcTestCase) { t.Fatalf("Error: PacketCapture Error Reason should be %v, but got %s", tc.expectedReason, pc.Status.Reason) } } - captured := pc.Status.NumCapturedPackets - if captured != tc.expectedNum { - if tc.expectedNum != 0 { - t.Fatalf("Error: PacketCapture captured packets count should be %v, but got %v", tc.expectedNum, captured) + if tc.expectedPhase == crdv1alpha1.PacketCaptureSucceeded { + captured := pc.Status.NumCapturedPackets + if captured != tc.expectedNum { + if tc.expectedNum != 0 { + t.Fatalf("Error: PacketCapture captured packets count should be %v, but got %v", tc.expectedNum, captured) + } } } + } func (data *TestData) waitForPacketCapture(t *testing.T, name string, phase crdv1alpha1.PacketCapturePhase) (*crdv1alpha1.PacketCapture, error) {