wgengine/filter/filtertype: make Match.IPProto a view #12526
Conversation
| } else { | ||
| m.IPProto = make([]ipproto.Proto, 0, len(r.IPProto)) | ||
| filtered := make([]ipproto.Proto, 0, len(r.IPProto)) |
There was a problem hiding this comment.
i guess we don't have a views.Filter or equivalent.
There was a problem hiding this comment.
Yeah, and we have slicex.Partition but it allocates a lot and doesn't support append. I'd like an AppendFilterFunc or something.
| @@ -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}), | |||
There was a problem hiding this comment.
i'd love a variant which could take variadic elements return a slice. like views.SliceFrom(ipproto.TCP, ipproto.UDP, ipproto.ICMPv4) (with a better name)
There was a problem hiding this comment.
Same. I can't think of a good name. And renaming SliceOf to SliceFrom would be too invasive, especially to third party code that might be using it.
There was a problem hiding this comment.
I like SliceOf for handing it an already existing slice. For variadic, SliceFrom kinda fits
bradfitz
force-pushed
the
bradfitz/ipproto_view
branch
from
9cdd49b
to
913b65b
Compare
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>
bradfitz
force-pushed
the
bradfitz/ipproto_view
branch
from
913b65b
to
22066f2
Compare
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)