Fix trie intermediate insert bug (#212)

routingtable/table_test.go

@@ -143,6 +143,95 @@ func TestAddPath(t *testing.T) {
 			},
 			expectedCount: 5,
 		},
+		{
+			name: "Insert disjunct prefixes plus one child high #2",
+			routes: []*route.Route{
+				route.NewRoute(net.NewPfx(net.IPv4FromOctets(10, 0, 0, 0), 12), nil),
+				route.NewRoute(net.NewPfx(net.IPv4FromOctets(10, 0, 0, 0), 10), nil),
+				route.NewRoute(net.NewPfx(net.IPv4FromOctets(11, 100, 123, 128), 25), nil),
+				route.NewRoute(net.NewPfx(net.IPv4FromOctets(10, 0, 0, 0), 8), nil),
+				route.NewRoute(net.NewPfx(net.IPv4FromOctets(11, 100, 123, 0), 24), nil),
+			},
+			expected: &node{
+				route: route.NewRoute(net.NewPfx(net.IPv4FromOctets(10, 0, 0, 0), 7), nil),
+				skip:  7,
+				dummy: true,
+				l: &node{
+					route: route.NewRoute(net.NewPfx(net.IPv4FromOctets(10, 0, 0, 0), 8), nil),
+					l: &node{
+						skip:  1,
+						route: route.NewRoute(net.NewPfx(net.IPv4FromOctets(10, 0, 0, 0), 10), nil),
+						l: &node{
+							skip:  1,
+							route: route.NewRoute(net.NewPfx(net.IPv4FromOctets(10, 0, 0, 0), 12), nil),
+						},
+					},
+				},
+				h: &node{
+					skip:  16,
+					route: route.NewRoute(net.NewPfx(net.IPv4FromOctets(11, 100, 123, 0), 24), nil),
+					h: &node{
+						skip:  0,
+						route: route.NewRoute(net.NewPfx(net.IPv4FromOctets(11, 100, 123, 128), 25), nil),
+					},
+				},
+			},
+			expectedCount: 5,
+		},
+		{
+			name: "Insert disjunct prefixes plus one child high #3",
+			routes: []*route.Route{
+				route.NewRoute(net.NewPfx(net.IPv4FromOctets(10, 0, 0, 0), 8), nil),
+				route.NewRoute(net.NewPfx(net.IPv4FromOctets(11, 100, 123, 0), 24), nil),
+				route.NewRoute(net.NewPfx(net.IPv4FromOctets(10, 0, 0, 0), 12), nil),
+				route.NewRoute(net.NewPfx(net.IPv4FromOctets(10, 0, 0, 0), 10), nil),
+				route.NewRoute(net.NewPfx(net.IPv4FromOctets(11, 100, 123, 128), 25), nil),
+			},
+			expected: &node{
+				route: route.NewRoute(net.NewPfx(net.IPv4FromOctets(10, 0, 0, 0), 7), nil),
+				skip:  7,
+				dummy: true,
+				l: &node{
+					route: route.NewRoute(net.NewPfx(net.IPv4FromOctets(10, 0, 0, 0), 8), nil),
+					l: &node{
+						skip:  1,
+						route: route.NewRoute(net.NewPfx(net.IPv4FromOctets(10, 0, 0, 0), 10), nil),
+						l: &node{
+							skip:  1,
+							route: route.NewRoute(net.NewPfx(net.IPv4FromOctets(10, 0, 0, 0), 12), nil),
+						},
+					},
+				},
+				h: &node{
+					skip:  16,
+					route: route.NewRoute(net.NewPfx(net.IPv4FromOctets(11, 100, 123, 0), 24), nil),
+					h: &node{
+						skip:  0,
+						route: route.NewRoute(net.NewPfx(net.IPv4FromOctets(11, 100, 123, 128), 25), nil),
+					},
+				},
+			},
+			expectedCount: 5,
+		},
+		{
+			name: "Insert triangle #2",
+			routes: []*route.Route{
+				route.NewRoute(net.NewPfx(net.IPv4FromOctets(10, 128, 0, 0), 9), nil),
+				route.NewRoute(net.NewPfx(net.IPv4FromOctets(10, 0, 0, 0), 8), nil),
+				route.NewRoute(net.NewPfx(net.IPv4FromOctets(10, 0, 0, 0), 9), nil),
+			},
+			expected: &node{
+				route: route.NewRoute(net.NewPfx(net.IPv4FromOctets(10, 0, 0, 0), 8), nil),
+				skip:  8,
+				l: &node{
+					route: route.NewRoute(net.NewPfx(net.IPv4FromOctets(10, 0, 0, 0), 9), nil),
+				},
+				h: &node{
+					route: route.NewRoute(net.NewPfx(net.IPv4FromOctets(10, 128, 0, 0), 9), nil),
+				},
+			},
+			expectedCount: 3,
+		},
 	}
 
 	for _, test := range tests {
@@ -151,8 +240,8 @@ func TestAddPath(t *testing.T) {
 			rt.AddPath(route.Prefix(), nil)
 		}
 
-		assert.Equal(t, test.expected, rt.root)
-		assert.Equal(t, test.expectedCount, rt.GetRouteCount())
+		assert.Equal(t, test.expected, rt.root, test.name)
+		assert.Equal(t, test.expectedCount, rt.GetRouteCount(), test.name)
 	}
 }
 

routingtable/trie.go

@@ -127,7 +127,7 @@ func (n *node) addPath(pfx net.Prefix, p *route.Path) (*node, bool) {
 	// is pfx NOT a subnet of this node?
 	if !currentPfx.Contains(pfx) {
 		if pfx.Contains(currentPfx) {
-			return n.insertBefore(pfx, p, n.route.Pfxlen()-n.skip-1), true
+			return n.insertBefore(pfx, p), true
 		}
 
 		return n.newSuperNode(pfx, p), true
@@ -147,6 +147,7 @@ func (n *node) insertLow(pfx net.Prefix, p *route.Path, parentPfxLen uint8) (*no
 		n.l = newNode(pfx, p, pfx.Pfxlen()-parentPfxLen-1, false)
 		return n, true
 	}
+
 	newRoot, isNew := n.l.addPath(pfx, p)
 	n.l = newRoot
 	return n, isNew
@@ -194,14 +195,14 @@ func (n *node) insertChildren(old *node, newPfx net.Prefix, newPath *route.Path)
 	}
 }
 
-func (n *node) insertBefore(pfx net.Prefix, p *route.Path, parentPfxLen uint8) *node {
+func (n *node) insertBefore(pfx net.Prefix, p *route.Path) *node {
 	tmp := n
 
 	pfxLenDiff := n.route.Pfxlen() - pfx.Pfxlen()
 	skip := n.skip - pfxLenDiff
 	new := newNode(pfx, p, skip, false)
 
-	b := pfx.Addr().BitAtPosition(parentPfxLen)
+	b := n.route.Prefix().Addr().BitAtPosition(pfx.Pfxlen() + 1)
 	if !b {
 		new.l = tmp
 		new.l.skip = tmp.route.Pfxlen() - pfx.Pfxlen() - 1