Repository: scionproto/scion
Pull request #3451 (2019-12-05 09:48:56)

Fix IPv6 registrations in godispatcher
The godispatcher uses two separate port tables for IPv4 and IPv6
addresses. While the lookup has selected the applicable table based on
the address type, the registration was always using the table for IPv4.

Also enhance the tests for UDPTable to check that a lookup actually
succeeds after a registration.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/scionproto/scion/3451)
<!-- Reviewable:end -->



Patch:
diff --git a/go/godispatcher/internal/registration/udptable.go b/go/godispatcher/internal/registration/udptable.go
index e249c0680..d2db76b11 100644
--- a/go/godispatcher/internal/registration/udptable.go
+++ b/go/godispatcher/internal/registration/udptable.go
@@ -97,10 +97,11 @@ func (t *UDPPortTable) computeAddressWithPort(address *net.UDPAddr) (*net.UDPAdd
 }
 
 func (t *UDPPortTable) insertUDPAddress(address *net.UDPAddr, value interface{}) {
-	ipTable, ok := t.v4PortTable[address.Port]
+	portTable := t.getPortTableByIP(address.IP)
+	ipTable, ok := portTable[address.Port]
 	if !ok {
 		ipTable = make(IPTable)
-		t.v4PortTable[address.Port] = ipTable
+		portTable[address.Port] = ipTable
 	}
 	ipTable[address.IP.String()] = value
 }


Test patch:
diff --git a/go/godispatcher/internal/registration/udptable_test.go b/go/godispatcher/internal/registration/udptable_test.go
index 956dbb0e2..a22e9f543 100644
--- a/go/godispatcher/internal/registration/udptable_test.go
+++ b/go/godispatcher/internal/registration/udptable_test.go
@@ -29,6 +29,12 @@ var minPort = 1024
 var maxPort = 65535
 
 func testUDPTableWithPorts(v4, v6 map[int]IPTable) *UDPPortTable {
+	if v4 == nil {
+		v4 = map[int]IPTable{}
+	}
+	if v6 == nil {
+		v6 = map[int]IPTable{}
+	}
 	return NewUDPPortTableFromMap(minPort, maxPort, v4, v6)
 }
 
@@ -118,25 +124,53 @@ func TestUDPPortTableInsert(t *testing.T) {
 	Convey("", t, func() {
 		Convey("Given an empty table", func() {
 			table := NewUDPPortTable(minPort, maxPort)
-			Convey("Inserting an address with a port returns a copy of the same address", func() {
-				address := &net.UDPAddr{IP: net.IP{10, 2, 3, 4}, Port: 10080}
-				retAddress, err := table.Insert(address, value)
-				SoMsg("err", err, ShouldBeNil)
-				SoMsg("address content", retAddress, ShouldResemble, address)
-				SoMsg("address not same object", retAddress, ShouldNotEqual, address)
-			})
-			Convey("Inserting an address with a 0 port returns an allocated port", func() {
-				address := &net.UDPAddr{IP: net.IP{10, 2, 3, 4}}
-				expectedAddress := &net.UDPAddr{IP: net.IP{10, 2, 3, 4}, Port: 1024}
-				retAddress, err := table.Insert(address, value)
-				SoMsg("err", err, ShouldBeNil)
-				SoMsg("address", retAddress, ShouldResemble, expectedAddress)
-			})
+			Convey("Inserting an IPv4 address with a port returns a copy of the same address",
+				func() {
+					address := &net.UDPAddr{IP: net.IP{10, 2, 3, 4}, Port: 10080}
+					retAddress, err := table.Insert(address, value)
+					_, lookupOk := table.Lookup(address)
+					SoMsg("err", err, ShouldBeNil)
+					SoMsg("address content", retAddress, ShouldResemble, address)
+					SoMsg("address not same object", retAddress, ShouldNotEqual, address)
+					SoMsg("lookup ok", lookupOk, ShouldBeTrue)
+				})
+			Convey("Inserting an IPv4 address with a 0 port returns an allocated port",
+				func() {
+					address := &net.UDPAddr{IP: net.IP{10, 2, 3, 4}}
+					expectedAddress := &net.UDPAddr{IP: net.IP{10, 2, 3, 4}, Port: 1024}
+					retAddress, err := table.Insert(address, value)
+					_, lookupOk := table.Lookup(expectedAddress)
+					SoMsg("err", err, ShouldBeNil)
+					SoMsg("address", retAddress, ShouldResemble, expectedAddress)
+					SoMsg("lookup ok", lookupOk, ShouldBeTrue)
+				})
+			Convey("Inserting an IPv6 address with a port returns a copy of the same address",
+				func() {
+					address := &net.UDPAddr{IP: docIPv6Address, Port: 10080}
+					retAddress, err := table.Insert(address, value)
+					_, lookupOk := table.Lookup(address)
+					SoMsg("err", err, ShouldBeNil)
+					SoMsg("address content", retAddress, ShouldResemble, address)
+					SoMsg("address not same object", retAddress, ShouldNotEqual, address)
+					SoMsg("lookup ok", lookupOk, ShouldBeTrue)
+				})
+			Convey("Inserting an IPv6 address with a 0 port returns an allocated port",
+				func() {
+					address := &net.UDPAddr{IP: docIPv6Address}
+					expectedAddress := &net.UDPAddr{IP: docIPv6Address, Port: 1024}
+					retAddress, err := table.Insert(address, value)
+					_, lookupOk := table.Lookup(expectedAddress)
+					SoMsg("err", err, ShouldBeNil)
+					SoMsg("address", retAddress, ShouldResemble, expectedAddress)
+					SoMsg("lookup ok", lookupOk, ShouldBeTrue)
+				})
 			Convey("Inserting an address without a value is not permitted", func() {
 				address := &net.UDPAddr{IP: net.IP{10, 2, 3, 4}, Port: 10080}
 				retAddress, err := table.Insert(address, nil)
+				_, lookupOk := table.Lookup(address)
 				SoMsg("err", err, ShouldNotBeNil)
 				SoMsg("address", retAddress, ShouldBeNil)
+				SoMsg("lookup ok", lookupOk, ShouldBeFalse)
 			})
 			Convey("Inserting a zero IPv4 address is permitted", func() {
 				address := &net.UDPAddr{IP: net.IPv4zero, Port: 10080}