From fa92dcb626cce061f7c540f5b0446c2516533141 Mon Sep 17 00:00:00 2001
From: Paul Chaignon <paul.chaignon@gmail.com>
Date: Tue, 5 Dec 2023 13:06:11 +0100
Subject: [PATCH] connectivity: Check for unexpected packet drops

Convert the check for missed tail call packet drops into a more general
check for any unexpected packets drops. Current expected packet drop
reasons are: Policy denied, Policy denied by denylist, and Unsupported
L3 protocol.

Contrary to the Missed tail call check who only ran in the context of
the conn-disrupt test, this new check is always run by default.

Signed-off-by: Paul Chaignon <paul.chaignon@gmail.com>
---
 connectivity/suite.go        |  4 ++--
 connectivity/tests/errors.go | 34 ++++++++++++----------------------
 2 files changed, 14 insertions(+), 24 deletions(-)

diff --git a/connectivity/suite.go b/connectivity/suite.go
index 37bbcacf2f..36a0e33fbf 100644
--- a/connectivity/suite.go
+++ b/connectivity/suite.go
@@ -252,10 +252,10 @@ func Run(ctx context.Context, ct *check.ConnectivityTest, addExtraTests func(*ch
 			// include --include-conn-disrupt-test"
 			return ct.Run(ctx)
 		}
-
-		ct.NewTest("no-missed-tail-calls").WithScenarios(tests.NoMissedTailCalls())
 	}
 
+	ct.NewTest("no-unexpected-packet-drops").WithScenarios(tests.NoUnexpectedPacketDrops())
+
 	// Run all tests without any policies in place.
 	noPoliciesScenarios := []check.Scenario{
 		tests.PodToPod(),
diff --git a/connectivity/tests/errors.go b/connectivity/tests/errors.go
index ec1009ae7c..63f67f7344 100644
--- a/connectivity/tests/errors.go
+++ b/connectivity/tests/errors.go
@@ -5,7 +5,6 @@ package tests
 
 import (
 	"context"
-	"strconv"
 	"strings"
 	"time"
 
@@ -75,43 +74,34 @@ func (n *noErrorsInLogs) Run(ctx context.Context, t *check.Test) {
 
 }
 
-// NoMissedTailCalls checks whether there were no drops due to missed (BPF)
-// tail calls.
-func NoMissedTailCalls() check.Scenario {
-	return &noMissedTailCalls{}
+// NoUnexpectedPacketDrops checks whether there were no drops due to expected
+// packet drops.
+func NoUnexpectedPacketDrops() check.Scenario {
+	return &noUnexpectedPacketDrops{}
 }
 
-type noMissedTailCalls struct{}
+type noUnexpectedPacketDrops struct{}
 
-func (n *noMissedTailCalls) Name() string {
-	return "no-missed-tail-calls"
+func (n *noUnexpectedPacketDrops) Name() string {
+	return "no-unexpected-packet-drops"
 }
 
-func (n *noMissedTailCalls) Run(ctx context.Context, t *check.Test) {
+func (n *noUnexpectedPacketDrops) Run(ctx context.Context, t *check.Test) {
 	ct := t.Context()
 	cmd := []string{
 		"/bin/sh", "-c",
-		"cilium metrics list -o json | jq '.[] | select( .name == \"cilium_drop_count_total\" and .labels.reason == \"Missed tail call\" ).value'",
+		"cilium metrics list -o json | jq '.[] | select((.name == \"cilium_datapath_drop_count_total\") and (.labels.reason | IN(\"Policy denied\", \"Policy denied by denylist\", \"Unsupported L3 protocol\") | not))'",
 	}
 
 	for _, pod := range ct.CiliumPods() {
 		pod := pod
 		stdout, err := pod.K8sClient.ExecInPod(ctx, pod.Pod.Namespace, pod.Pod.Name, defaults.AgentContainerName, cmd)
 		if err != nil {
-			t.Fatalf("Error fetching missed tail call drop counts: %s", err)
+			t.Fatalf("Error fetching packet drop counts: %s", err)
 		}
 		countStr := strings.TrimSpace(stdout.String())
-		if countStr == "" {
-			return
-		}
-
-		count, err := strconv.Atoi(countStr)
-		if err != nil {
-			t.Fatalf("Failed to convert missed tail call drops %q to int: %s", countStr, err)
-		}
-
-		if count != 0 {
-			t.Fatalf("Detected drops due to missed tail calls: %d", count)
+		if countStr != "" {
+			t.Fatalf("Found unexpected packet drops:\n%s", countStr)
 		}
 	}