wgengine/filter/filtertype: make Match.IPProto a view

I noticed we were allocating these every time when they could just
share the same memory. Rather than document ownership, just lock it
down with a view.

I was considering doing all of the fields but decided to just do this
one first as test to see how infectious it became.  Conclusion: not
very.

Updates #cleanup (while working towards tailscale/corp#20514)

Change-Id: I8ce08519de0c9a53f20292adfbecd970fe362de0
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
This commit is contained in:
Brad Fitzpatrick 2024-06-18 12:05:34 -07:00 committed by Brad Fitzpatrick
parent bfb775ce62
commit bd93c3067e
8 changed files with 26 additions and 24 deletions

View File

@ -36,6 +36,7 @@
"tailscale.com/types/logger"
"tailscale.com/types/netlogtype"
"tailscale.com/types/ptr"
"tailscale.com/types/views"
"tailscale.com/util/must"
"tailscale.com/wgengine/capture"
"tailscale.com/wgengine/filter"
@ -156,10 +157,10 @@ func netports(netPorts ...string) (ret []filter.NetPortRange) {
}
func setfilter(logf logger.Logf, tun *Wrapper) {
protos := []ipproto.Proto{
protos := views.SliceOf([]ipproto.Proto{
ipproto.TCP,
ipproto.UDP,
}
})
matches := []filter.Match{
{IPProto: protos, Srcs: nets("5.6.7.8"), Dsts: netports("1.2.3.4:89-90")},
{IPProto: protos, Srcs: nets("1.2.3.4"), Dsts: netports("5.6.7.8:98")},

View File

@ -27,6 +27,7 @@
"tailscale.com/types/ipproto"
"tailscale.com/types/key"
"tailscale.com/types/ptr"
"tailscale.com/types/views"
"tailscale.com/util/deephash/testtype"
"tailscale.com/util/dnsname"
"tailscale.com/util/hashx"
@ -353,7 +354,7 @@ func getVal() *tailscaleTypes {
},
},
filter.Match{
IPProto: []ipproto.Proto{1, 2, 3},
IPProto: views.SliceOf([]ipproto.Proto{1, 2, 3}),
},
}
}

View File

@ -126,7 +126,7 @@ func NewAllowAllForTest(logf logger.Logf) *Filter {
any6 := netip.PrefixFrom(netip.AddrFrom16([16]byte{}), 0)
ms := []Match{
{
IPProto: []ipproto.Proto{ipproto.TCP, ipproto.UDP, ipproto.ICMPv4},
IPProto: views.SliceOf([]ipproto.Proto{ipproto.TCP, ipproto.UDP, ipproto.ICMPv4}),
Srcs: []netip.Prefix{any4},
Dsts: []NetPortRange{
{
@ -139,7 +139,7 @@ func NewAllowAllForTest(logf logger.Logf) *Filter {
},
},
{
IPProto: []ipproto.Proto{ipproto.TCP, ipproto.UDP, ipproto.ICMPv6},
IPProto: views.SliceOf([]ipproto.Proto{ipproto.TCP, ipproto.UDP, ipproto.ICMPv6}),
Srcs: []netip.Prefix{any6},
Dsts: []NetPortRange{
{

View File

@ -45,7 +45,7 @@ func m(srcs []netip.Prefix, dsts []NetPortRange, protos ...ipproto.Proto) Match
protos = defaultProtos
}
return Match{
IPProto: protos,
IPProto: views.SliceOf(protos),
Srcs: srcs,
SrcsContains: ipset.NewContainsIPFunc(views.SliceOf(srcs)),
Dsts: dsts,
@ -767,12 +767,7 @@ func TestMatchesFromFilterRules(t *testing.T) {
},
want: []Match{
{
IPProto: []ipproto.Proto{
ipproto.TCP,
ipproto.UDP,
ipproto.ICMPv4,
ipproto.ICMPv6,
},
IPProto: defaultProtosView,
Dsts: []NetPortRange{
{
Net: netip.MustParsePrefix("0.0.0.0/0"),
@ -804,9 +799,9 @@ func TestMatchesFromFilterRules(t *testing.T) {
},
want: []Match{
{
IPProto: []ipproto.Proto{
IPProto: views.SliceOf([]ipproto.Proto{
ipproto.TCP,
},
}),
Dsts: []NetPortRange{
{
Net: netip.MustParsePrefix("1.2.0.0/16"),
@ -830,6 +825,7 @@ func TestMatchesFromFilterRules(t *testing.T) {
cmpOpts := []cmp.Option{
cmp.Comparer(func(a, b netip.Addr) bool { return a == b }),
cmp.Comparer(func(a, b netip.Prefix) bool { return a == b }),
cmp.Comparer(func(a, b views.Slice[ipproto.Proto]) bool { return views.SliceEqual(a, b) }),
cmpopts.IgnoreFields(Match{}, ".SrcsContains"),
}
if diff := cmp.Diff(got, tt.want, cmpOpts...); diff != "" {

View File

@ -11,6 +11,7 @@
"tailscale.com/tailcfg"
"tailscale.com/types/ipproto"
"tailscale.com/types/views"
)
//go:generate go run tailscale.com/cmd/cloner --type=Match,CapMatch
@ -65,7 +66,7 @@ type CapMatch struct {
// Match matches packets from any IP address in Srcs to any ip:port in
// Dsts.
type Match struct {
IPProto []ipproto.Proto // required set (no default value at this layer)
IPProto views.Slice[ipproto.Proto] // required set (no default value at this layer)
Srcs []netip.Prefix
SrcsContains func(netip.Addr) bool `json:"-"` // report whether Addr is in Srcs
Dsts []NetPortRange // optional, if Srcs match

View File

@ -10,6 +10,7 @@
"tailscale.com/tailcfg"
"tailscale.com/types/ipproto"
"tailscale.com/types/views"
)
// Clone makes a deep copy of Match.
@ -20,7 +21,7 @@ func (src *Match) Clone() *Match {
}
dst := new(Match)
*dst = *src
dst.IPProto = append(src.IPProto[:0:0], src.IPProto...)
dst.IPProto = src.IPProto
dst.Srcs = append(src.Srcs[:0:0], src.Srcs...)
dst.Dsts = append(src.Dsts[:0:0], src.Dsts...)
if src.Caps != nil {
@ -34,7 +35,7 @@ func (src *Match) Clone() *Match {
// A compilation failure here means this code must be regenerated, with the command at the top of this file.
var _MatchCloneNeedsRegeneration = Match(struct {
IPProto []ipproto.Proto
IPProto views.Slice[ipproto.Proto]
Srcs []netip.Prefix
SrcsContains func(netip.Addr) bool
Dsts []NetPortRange

View File

@ -4,9 +4,8 @@
package filter
import (
"slices"
"tailscale.com/net/packet"
"tailscale.com/types/views"
"tailscale.com/wgengine/filter/filtertype"
)
@ -14,7 +13,7 @@
func (ms matches) match(q *packet.Parsed) bool {
for _, m := range ms {
if !slices.Contains(m.IPProto, q.IPProto) {
if !views.SliceContains(m.IPProto, q.IPProto) {
continue
}
if !m.SrcsContains(q.Src.Addr()) {
@ -52,7 +51,7 @@ func (ms matches) matchIPsOnly(q *packet.Parsed) bool {
// ignored, as long as the match is for the entire uint16 port range.
func (ms matches) matchProtoAndIPsOnlyIfAllPorts(q *packet.Parsed) bool {
for _, m := range ms {
if !slices.Contains(m.IPProto, q.IPProto) {
if !views.SliceContains(m.IPProto, q.IPProto) {
continue
}
if !m.SrcsContains(q.Src.Addr()) {

View File

@ -23,6 +23,8 @@
ipproto.ICMPv6,
}
var defaultProtosView = views.SliceOf(defaultProtos)
// MatchesFromFilterRules converts tailcfg FilterRules into Matches.
// If an error is returned, the Matches result is still valid,
// containing the rules that were successfully converted.
@ -41,14 +43,15 @@ func MatchesFromFilterRules(pf []tailcfg.FilterRule) ([]Match, error) {
}
if len(r.IPProto) == 0 {
m.IPProto = append([]ipproto.Proto(nil), defaultProtos...)
m.IPProto = defaultProtosView
} else {
m.IPProto = make([]ipproto.Proto, 0, len(r.IPProto))
filtered := make([]ipproto.Proto, 0, len(r.IPProto))
for _, n := range r.IPProto {
if n >= 0 && n <= 0xff {
m.IPProto = append(m.IPProto, ipproto.Proto(n))
filtered = append(filtered, ipproto.Proto(n))
}
}
m.IPProto = views.SliceOf(filtered)
}
for i, s := range r.SrcIPs {