wgengine/filter: return drop reason for metrics

Updates #14280

Signed-off-by: Kristoffer Dalby <kristoffer@tailscale.com>

wgengine/filter/filter.go

@@ -24,6 +24,7 @@ import (
 	"tailscale.com/types/views"
 	"tailscale.com/util/mak"
 	"tailscale.com/util/slicesx"
+	"tailscale.com/util/usermetric"
 	"tailscale.com/wgengine/filter/filtertype"
 )
 
@@ -410,7 +411,7 @@ func (f *Filter) ShieldsUp() bool { return f.shieldsUp }
 // Tailscale peer.
 func (f *Filter) RunIn(q *packet.Parsed, rf RunFlags) Response {
 	dir := in
-	r := f.pre(q, rf, dir)
+	r, _ := f.pre(q, rf, dir)
 	if r == Accept || r == Drop {
 		// already logged
 		return r
@@ -431,16 +432,16 @@ func (f *Filter) RunIn(q *packet.Parsed, rf RunFlags) Response {
 
 // RunOut determines whether this node is allowed to send q to a
 // Tailscale peer.
-func (f *Filter) RunOut(q *packet.Parsed, rf RunFlags) Response {
+func (f *Filter) RunOut(q *packet.Parsed, rf RunFlags) (Response, usermetric.DropReason) {
 	dir := out
-	r := f.pre(q, rf, dir)
+	r, reason := f.pre(q, rf, dir)
 	if r == Accept || r == Drop {
 		// already logged
-		return r
+		return r, reason
 	}
 	r, why := f.runOut(q)
 	f.logRateLimit(rf, q, dir, r, why)
-	return r
+	return r, ""
 }
 
 var unknownProtoStringCache sync.Map // ipproto.Proto -> string
@@ -610,33 +611,33 @@ var gcpDNSAddr = netaddr.IPv4(169, 254, 169, 254)
 
 // pre runs the direction-agnostic filter logic. dir is only used for
 // logging.
-func (f *Filter) pre(q *packet.Parsed, rf RunFlags, dir direction) Response {
+func (f *Filter) pre(q *packet.Parsed, rf RunFlags, dir direction) (Response, usermetric.DropReason) {
 	if len(q.Buffer()) == 0 {
 		// wireguard keepalive packet, always permit.
-		return Accept
+		return Accept, ""
 	}
 	if len(q.Buffer()) < 20 {
 		f.logRateLimit(rf, q, dir, Drop, "too short")
-		return Drop
+		return Drop, usermetric.ReasonTooShort
 	}
 
 	if q.Dst.Addr().IsMulticast() {
 		f.logRateLimit(rf, q, dir, Drop, "multicast")
-		return Drop
+		return Drop, usermetric.ReasonMulticast
 	}
 	if q.Dst.Addr().IsLinkLocalUnicast() && q.Dst.Addr() != gcpDNSAddr {
 		f.logRateLimit(rf, q, dir, Drop, "link-local-unicast")
-		return Drop
+		return Drop, usermetric.ReasonLinkLocalUnicast
 	}
 
 	if q.IPProto == ipproto.Fragment {
 		// Fragments after the first always need to be passed through.
 		// Very small fragments are considered Junk by Parsed.
 		f.logRateLimit(rf, q, dir, Accept, "fragment")
-		return Accept
+		return Accept, ""
 	}
 
-	return noVerdict
+	return noVerdict, ""
 }
 
 // loggingAllowed reports whether p can appear in logs at all.

wgengine/filter/filter_test.go

@@ -30,6 +30,7 @@ import (
 	"tailscale.com/types/views"
 	"tailscale.com/util/must"
 	"tailscale.com/util/slicesx"
+	"tailscale.com/util/usermetric"
 	"tailscale.com/wgengine/filter/filtertype"
 )
 
@@ -211,7 +212,7 @@ func TestUDPState(t *testing.T) {
 		t.Fatalf("incoming initial packet not dropped, got=%v: %v", got, a4)
 	}
 	// We talk to that peer
-	if got := acl.RunOut(&b4, flags); got != Accept {
+	if got, _ := acl.RunOut(&b4, flags); got != Accept {
 		t.Fatalf("outbound packet didn't egress, got=%v: %v", got, b4)
 	}
 	// Now, the same packet as before is allowed back.
@@ -227,7 +228,7 @@ func TestUDPState(t *testing.T) {
 		t.Fatalf("incoming initial packet not dropped: %v", a4)
 	}
 	// We talk to that peer
-	if got := acl.RunOut(&b6, flags); got != Accept {
+	if got, _ := acl.RunOut(&b6, flags); got != Accept {
 		t.Fatalf("outbound packet didn't egress: %v", b4)
 	}
 	// Now, the same packet as before is allowed back.
@@ -382,25 +383,26 @@ func BenchmarkFilter(b *testing.B) {
 
 func TestPreFilter(t *testing.T) {
 	packets := []struct {
-		desc string
+		desc       string
-		want Response
+		want       Response
-		b    []byte
+		wantReason usermetric.DropReason
+		b          []byte
 	}{
-		{"empty", Accept, []byte{}},
+		{"empty", Accept, "", []byte{}},
-		{"short", Drop, []byte("short")},
+		{"short", Drop, usermetric.ReasonTooShort, []byte("short")},
-		{"junk", Drop, raw4default(ipproto.Unknown, 10)},
+		{"junk", Drop, "", raw4default(ipproto.Unknown, 10)},
-		{"fragment", Accept, raw4default(ipproto.Fragment, 40)},
+		{"fragment", Accept, "", raw4default(ipproto.Fragment, 40)},
-		{"tcp", noVerdict, raw4default(ipproto.TCP, 0)},
+		{"tcp", noVerdict, "", raw4default(ipproto.TCP, 0)},
-		{"udp", noVerdict, raw4default(ipproto.UDP, 0)},
+		{"udp", noVerdict, "", raw4default(ipproto.UDP, 0)},
-		{"icmp", noVerdict, raw4default(ipproto.ICMPv4, 0)},
+		{"icmp", noVerdict, "", raw4default(ipproto.ICMPv4, 0)},
 	}
 	f := NewAllowNone(t.Logf, &netipx.IPSet{})
 	for _, testPacket := range packets {
 		p := &packet.Parsed{}
 		p.Decode(testPacket.b)
-		got := f.pre(p, LogDrops|LogAccepts, in)
+		got, gotReason := f.pre(p, LogDrops|LogAccepts, in)
-		if got != testPacket.want {
+		if got != testPacket.want || gotReason != testPacket.wantReason {
-			t.Errorf("%q got=%v want=%v packet:\n%s", testPacket.desc, got, testPacket.want, packet.Hexdump(testPacket.b))
+			t.Errorf("%q got=%v want=%v gotReason=%s wantReason=%s packet:\n%s", testPacket.desc, got, testPacket.want, gotReason, testPacket.wantReason, packet.Hexdump(testPacket.b))
 		}
 	}
 }