fix: group observations by zeroing port

In #917, we started dropping additional address observations if we had multiple
for the same transport set. However, on further consideration, this isn't quite
correct. We _want_ to keep additional observations for multiple IP addresses.
The real issue is many observations for different ports.

So this patch simply changes the key with which we group observations from
"address protocols" to "address without the port" (well, with the port set to
0).
This commit is contained in:
Steven Allen 2020-05-20 11:35:49 -07:00
parent b396e69aee
commit 6ef5f5dacf
2 changed files with 39 additions and 19 deletions

View File

@ -70,15 +70,24 @@ func (oa *ObservedAddr) numInbound() int {
return count
}
// GroupKey returns the group in which this observation belongs. Currently, an
// observed address's group is just the address with all ports set to 0. This
// means we can advertise the most commonly observed external ports without
// advertising _every_ observed port.
func (oa *ObservedAddr) GroupKey() string {
key := ""
protos := oa.Addr.Protocols()
key := make([]byte, 0, len(oa.Addr.Bytes()))
ma.ForEach(oa.Addr, func(c ma.Component) bool {
switch proto := c.Protocol(); proto.Code {
case ma.P_TCP, ma.P_UDP:
key = append(key, proto.VCode...)
key = append(key, 0, 0) // zero in two bytes
default:
key = append(key, c.Bytes()...)
}
return true
})
for i := range protos {
key = key + "/" + protos[i].Name
}
return key
return string(key)
}
type newObservation struct {

View File

@ -314,17 +314,28 @@ func TestObservedAddrFiltering(t *testing.T) {
}
func TestObservedAddrGroupKey(t *testing.T) {
oa1 := &identify.ObservedAddr{Addr: ma.StringCast("/ip4/1.2.3.4/tcp/1231")}
require.Equal(t, "/ip4/tcp", oa1.GroupKey())
oa1 := &identify.ObservedAddr{Addr: ma.StringCast("/ip4/1.2.3.4/tcp/2345")}
oa2 := &identify.ObservedAddr{Addr: ma.StringCast("/ip4/1.2.3.4/tcp/1231")}
oa3 := &identify.ObservedAddr{Addr: ma.StringCast("/ip4/1.2.3.5/tcp/1231")}
oa4 := &identify.ObservedAddr{Addr: ma.StringCast("/ip4/1.2.3.4/udp/1231")}
oa5 := &identify.ObservedAddr{Addr: ma.StringCast("/ip4/1.2.3.4/udp/1531")}
oa6 := &identify.ObservedAddr{Addr: ma.StringCast("/ip4/1.2.3.4/udp/1531/quic")}
oa7 := &identify.ObservedAddr{Addr: ma.StringCast("/ip4/1.2.3.4/udp/1111/quic")}
oa8 := &identify.ObservedAddr{Addr: ma.StringCast("/ip4/1.2.3.5/udp/1111/quic")}
oa2 := &identify.ObservedAddr{Addr: ma.StringCast("/ip4/1.2.3.5/tcp/2222")}
require.Equal(t, "/ip4/tcp", oa2.GroupKey())
oa3 := &identify.ObservedAddr{Addr: ma.StringCast("/ip4/1.2.3.4/udp/1231")}
require.Equal(t, "/ip4/udp", oa3.GroupKey())
oa4 := &identify.ObservedAddr{Addr: ma.StringCast("/ip4/1.3.3.4/udp/1531")}
require.Equal(t, "/ip4/udp", oa4.GroupKey())
oa5 := &identify.ObservedAddr{Addr: ma.StringCast("/ip4/1.3.3.4/udp/1531/quic")}
require.Equal(t, "/ip4/udp/quic", oa5.GroupKey())
// different ports, same IP => same key
require.Equal(t, oa1.GroupKey(), oa2.GroupKey())
// different IPs => different key
require.NotEqual(t, oa2.GroupKey(), oa3.GroupKey())
// same port, different protos => different keys
require.NotEqual(t, oa3.GroupKey(), oa4.GroupKey())
// same port, same address, different protos => different keys
require.NotEqual(t, oa2.GroupKey(), oa4.GroupKey())
// udp works as well
require.Equal(t, oa4.GroupKey(), oa5.GroupKey())
// udp and quic are different
require.NotEqual(t, oa5.GroupKey(), oa6.GroupKey())
// quic works as well
require.Equal(t, oa6.GroupKey(), oa7.GroupKey())
require.NotEqual(t, oa7.GroupKey(), oa8.GroupKey())
}