bpf: add metrics for fragmented IPv4 packets

This commit introduces 2 new metrics in the datapath logic related to
fragmented IPv4 packets:

* `REASON_FRAG_PACKET`: number of received fragmented packets
* `REASON_FRAG_PACKET_UPDATE`: number of failures in updating the
  `IPV4_FRAG_DATAGRAMS_MAP` map to register the first logical fragment of
  a datagram

Fixes: #11179

Signed-off-by: Gilberto Bertin <gilberto@isovalent.com>

api/v1/flow/flow.proto

@@ -226,8 +226,9 @@ enum Verdict {
     ERROR = 3;
 }
 
-// Taken from pkg/monitor/api/drop.go. Note that non-drop reasons (i.e. values
-// less than api.DropMin) are not used here.
+// These values are shared with pkg/monitor/api/drop.go and bpf/lib/common.h.
+// Note that non-drop reasons (i.e. values less than api.DropMin) are not used
+// here.
 enum DropReason {
     // non-drop reasons
     DROP_REASON_UNKNOWN = 0;

bpf/lib/common.h

@@ -328,6 +328,9 @@ enum {
 /* Cilium error codes, must NOT overlap with TC return codes.
  * These also serve as drop reasons for metrics,
  * where reason > 0 corresponds to -(DROP_*)
+ *
+ * These are shared with pkg/monitor/api/drop.go and api/v1/flow/flow.proto.
+ * When modifying any of the below, those files should also be updated.
  */
 #define DROP_UNUSED1		-130 /* unused */
 #define DROP_UNUSED2		-131 /* unused */
@@ -387,6 +390,9 @@ enum {
 /* Cilium metrics reasons for forwarding packets and other stats.
  * If reason is larger than below then this is a drop reason and
  * value corresponds to -(DROP_*), see above.
+ *
+ * These are shared with pkg/monitor/api/drop.go.
+ * When modifying any of the below, those files should also be updated.
  */
 #define REASON_FORWARDED		0
 #define REASON_PLAINTEXT		3
@@ -395,6 +401,8 @@ enum {
 #define REASON_LB_NO_BACKEND		6
 #define REASON_LB_REVNAT_UPDATE		7
 #define REASON_LB_REVNAT_STALE		8
+#define REASON_FRAG_PACKET		9
+#define REASON_FRAG_PACKET_UPDATE	10
 
 /* Lookup scope for externalTrafficPolicy=Local */
 #define LB_LOOKUP_SCOPE_EXT	0

bpf/lib/conntrack.h

@@ -517,6 +517,7 @@ ipv4_ct_tuple_reverse(struct ipv4_ct_tuple *tuple)
 
 static __always_inline int ipv4_ct_extract_l4_ports(struct __ctx_buff *ctx,
 						    int off,
+						    int dir __maybe_unused,
 						    struct ipv4_ct_tuple *tuple,
 						    bool *has_l4_header __maybe_unused)
 {
@@ -531,7 +532,7 @@ static __always_inline int ipv4_ct_extract_l4_ports(struct __ctx_buff *ctx,
 		return DROP_CT_INVALID_HDR;
 
 	if (unlikely(ipv4_is_fragment(ip4)))
-		return ipv4_handle_fragment(ctx, ip4, off,
+		return ipv4_handle_fragment(ctx, ip4, off, dir,
 					    (struct ipv4_frag_l4ports *)&tuple->dport,
 					    has_l4_header);
 #endif
@@ -616,7 +617,7 @@ static __always_inline int ct_lookup4(const void *map,
 		break;
 
 	case IPPROTO_TCP:
-		err = ipv4_ct_extract_l4_ports(ctx, off, tuple, &has_l4_header);
+		err = ipv4_ct_extract_l4_ports(ctx, off, dir, tuple, &has_l4_header);
 		if (err < 0)
 			return err;
 
@@ -632,7 +633,7 @@ static __always_inline int ct_lookup4(const void *map,
 		break;
 
 	case IPPROTO_UDP:
-		err = ipv4_ct_extract_l4_ports(ctx, off, tuple, NULL);
+		err = ipv4_ct_extract_l4_ports(ctx, off, dir, tuple, NULL);
 		if (err < 0)
 			return err;
 

bpf/lib/ipv4.h

@@ -7,6 +7,7 @@
 #include <linux/ip.h>
 
 #include "dbg.h"
+#include "metrics.h"
 
 struct ipv4_frag_id {
 	__be32	daddr;
@@ -107,7 +108,7 @@ ipv4_frag_get_l4ports(const struct ipv4_frag_id *frag_id,
 }
 
 static __always_inline int
-ipv4_frag_register_datagram(struct __ctx_buff *ctx, int l4_off,
+ipv4_frag_register_datagram(struct __ctx_buff *ctx, int l4_off, int dir,
 			    const struct ipv4_frag_id *frag_id,
 			    struct ipv4_frag_l4ports *ports)
 {
@@ -117,7 +118,9 @@ ipv4_frag_register_datagram(struct __ctx_buff *ctx, int l4_off,
 	if (ret < 0)
 		return ret;
 
-	map_update_elem(&IPV4_FRAG_DATAGRAMS_MAP, frag_id, ports, BPF_ANY);
+	if (map_update_elem(&IPV4_FRAG_DATAGRAMS_MAP, frag_id, ports, BPF_ANY))
+		update_metrics(ctx_full_len(ctx), dir, REASON_FRAG_PACKET_UPDATE);
+
 	/* Do not return an error if map update failed, as nothing prevents us
 	 * to process the current packet normally.
 	 */
@@ -126,7 +129,7 @@ ipv4_frag_register_datagram(struct __ctx_buff *ctx, int l4_off,
 
 static __always_inline int
 ipv4_handle_fragment(struct __ctx_buff *ctx,
-		     const struct iphdr *ip4, int l4_off,
+		     const struct iphdr *ip4, int l4_off, int dir,
 		     struct ipv4_frag_l4ports *ports,
 		     bool *has_l4_header)
 {
@@ -140,6 +143,8 @@ ipv4_handle_fragment(struct __ctx_buff *ctx,
 		.pad = 0,
 	};
 
+	update_metrics(ctx_full_len(ctx), dir, REASON_FRAG_PACKET);
+
 	not_first_fragment = ipv4_is_not_first_fragment(ip4);
 	if (has_l4_header)
 		*has_l4_header = !not_first_fragment;
@@ -151,7 +156,7 @@ ipv4_handle_fragment(struct __ctx_buff *ctx,
 	 * we receive). Fragment has L4 header, we can retrieve L4 ports and
 	 * create an entry in datagrams map.
 	 */
-	return ipv4_frag_register_datagram(ctx, l4_off, &frag_id, ports);
+	return ipv4_frag_register_datagram(ctx, l4_off, dir, &frag_id, ports);
 }
 #endif
 

bpf/lib/lb.h

@@ -340,7 +340,9 @@ bool lb4_svc_is_localredirect(const struct lb4_service *svc __maybe_unused)
 }
 
 static __always_inline int extract_l4_port(struct __ctx_buff *ctx, __u8 nexthdr,
-					   int l4_off, __be16 *port,
+					   int l4_off,
+					   int dir __maybe_unused,
+					   __be16 *port,
 					   __maybe_unused struct iphdr *ip4)
 {
 	int ret;
@@ -354,7 +356,7 @@ static __always_inline int extract_l4_port(struct __ctx_buff *ctx, __u8 nexthdr,
 
 			if (unlikely(ipv4_is_fragment(ip4))) {
 				ret = ipv4_handle_fragment(ctx, ip4, l4_off,
-							   &ports,
+							   dir, &ports,
 							   NULL);
 				if (IS_ERR(ret))
 					return ret;
@@ -513,7 +515,8 @@ static __always_inline int lb6_extract_key(struct __ctx_buff *ctx __maybe_unused
 	ipv6_addr_copy(&key->address, addr);
 	csum_l4_offset_and_flags(tuple->nexthdr, csum_off);
 
-	return extract_l4_port(ctx, tuple->nexthdr, l4_off, &key->dport, NULL);
+	return extract_l4_port(ctx, tuple->nexthdr, l4_off, dir, &key->dport,
+			       NULL);
 }
 
 static __always_inline
@@ -1033,7 +1036,7 @@ static __always_inline int lb4_extract_key(struct __ctx_buff *ctx __maybe_unused
 	if (ipv4_has_l4_header(ip4))
 		csum_l4_offset_and_flags(ip4->protocol, csum_off);
 
-	return extract_l4_port(ctx, ip4->protocol, l4_off, &key->dport, ip4);
+	return extract_l4_port(ctx, ip4->protocol, l4_off, dir, &key->dport, ip4);
 }
 
 static __always_inline

bpf/tests/ipv6_test.h

@@ -2,8 +2,6 @@
 /* Copyright (C) 2018-2020 Authors of Cilium */
 
 #include "lib/ipv6.h"
-
-#define SKIP_UNDEF_LPM_LOOKUP_FN
 #include "lib/maps.h"
 
 static void test_ipv6_addr_clear_suffix(void)

pkg/monitor/api/drop.go

@@ -24,6 +24,7 @@ var DropMin uint8 = 130
 // DropInvalid is the Invalid packet reason.
 var DropInvalid uint8 = 2
 
+// These values are shared with bpf/lib/common.h and api/v1/flow/flow.proto.
 var errors = map[uint8]string{
 	0:   "Success",
 	2:   "Invalid packet",
@@ -33,6 +34,8 @@ var errors = map[uint8]string{
 	6:   "LB, sock cgroup: No backend entry found",
 	7:   "LB, sock cgroup: Reverse entry update failed",
 	8:   "LB, sock cgroup: Reverse entry stale",
+	9:   "Fragmented packet",
+	10:  "Fragmented packet entry update failed",
 	130: "Invalid source mac",      // Unused
 	131: "Invalid destination mac", // Unused
 	132: "Invalid source ip",

test/bpf/unit-test.c

@@ -8,6 +8,14 @@
 #include <stdlib.h>
 
 #include "lib/utils.h"
+
+/* SKIP_UNDEF_LPM_LOOKUP_FN is used to control if the LPM_LOOKUP_FN macro in
+ * lib/maps.h should be defined or not.
+ *
+ * As lib/common.h includes in turn lib/maps.h, define SKIP_UNDEF_LPM_LOOKUP_FN
+ * here since unit tests require the LPM_LOOKUP_FN macro to be defined.
+ */
+#define SKIP_UNDEF_LPM_LOOKUP_FN
 #include "lib/common.h"
 
 #include "node_config.h"

test/helpers/wrappers.go

@@ -15,7 +15,9 @@
 package helpers
 
 import (
+	"context"
 	"fmt"
+	"strconv"
 	"strings"
 )
 
@@ -197,3 +199,12 @@ func OpenSSLShowCerts(host string, port uint16, serverName string) string {
 	}
 	return fmt.Sprintf("openssl s_client -connect %s:%d %s -showcerts | openssl x509 -outform PEM", host, port, serverNameFlag)
 }
+
+// GetBPFPacketsCount returns the number of packets for a given drop reason and
+// direction by parsing BPF metrics.
+func GetBPFPacketsCount(kubectl *Kubectl, pod, reason, direction string) (int, error) {
+	cmd := fmt.Sprintf("cilium bpf metrics list | awk '/%s *%s/ {print $4}'", reason, direction)
+	res := kubectl.CiliumExecMustSucceed(context.TODO(), pod, cmd)
+
+	return strconv.Atoi(strings.TrimSpace(res.Stdout()))
+}

test/k8sT/Services.go

@@ -753,7 +753,7 @@ var _ = Describe("K8sServicesTest", func() {
 		}
 
 		// srcPod:     Name of pod sending the datagram
-		// srcPort:    Source UDP port
+		// srcPort:    Source UDP port (should be different for each doFragmentRequest invocation to allow distinct CT table entries)
 		// dstPodIP:   Receiver pod IP (for checking in CT table)
 		// dstPodPort: Receiver pod port (for checking in CT table)
 		// dstIP:      Target endpoint IP for sending the datagram
@@ -818,6 +818,9 @@ var _ = Describe("K8sServicesTest", func() {
 				countOutK8s2, _ = strconv.Atoi(strings.TrimSpace(res.Stdout()))
 			}
 
+			fragmentedPacketsBeforeK8s1, _ := helpers.GetBPFPacketsCount(kubectl, ciliumPodK8s1, "Fragmented packet", "INGRESS")
+			fragmentedPacketsBeforeK8s2, _ := helpers.GetBPFPacketsCount(kubectl, ciliumPodK8s2, "Fragmented packet", "INGRESS")
+
 			// Send datagram
 			By("Sending a fragmented packet from %s to endpoint %s", srcPod, net.JoinHostPort(dstIP, fmt.Sprintf("%d", dstPort)))
 			cmd := fmt.Sprintf("bash -c 'dd if=/dev/zero bs=%d count=%d | nc -u -w 1 -p %d %s %d'", blockSize, blockCount, srcPort, dstIP, dstPort)
@@ -863,6 +866,14 @@ var _ = Describe("K8sServicesTest", func() {
 				Equal([]int{countOutK8s1, countOutK8s2 + delta}),
 				Equal([]int{countOutK8s1 + delta, countOutK8s2}),
 			), "Failed to account for IPv4 fragments to %s (out)", dstIP)
+
+			fragmentedPacketsAfterK8s1, _ := helpers.GetBPFPacketsCount(kubectl, ciliumPodK8s1, "Fragmented packet", "INGRESS")
+			fragmentedPacketsAfterK8s2, _ := helpers.GetBPFPacketsCount(kubectl, ciliumPodK8s2, "Fragmented packet", "INGRESS")
+
+			ExpectWithOffset(2, []int{fragmentedPacketsAfterK8s1, fragmentedPacketsAfterK8s2}).To(SatisfyAny(
+				Equal([]int{fragmentedPacketsBeforeK8s1, fragmentedPacketsBeforeK8s2 + delta}),
+				Equal([]int{fragmentedPacketsBeforeK8s1 + delta, fragmentedPacketsBeforeK8s2}),
+			), "Failed to account for INGRESS IPv4 fragments in BPF metrics", dstIP)
 		}
 
 		getIPv4AddrForIface := func(nodeName, iface string) string {