Fixed dual stack bridge

+ better error messages for dual stack objects

Author: hknutzen

Changes

@@ -1,5 +1,8 @@
 {{$NEXT}}
 
+ - Fixed dual stack bridge. IPv4 and IPv6 interfaces were accidently
+   processed together.
+
 6.075     2024-12-19 11:53:37+01:00 Europe/Berlin
 
  - Fix: Attribute 'policy_distribution_point' at dual stack router

go/pkg/pass1/check-ip.go

@@ -1,9 +1,7 @@
 package pass1
 
 import (
-	"maps"
 	"net/netip"
-	"slices"
 	"strings"
 
 	"go4.org/netipx"
@@ -53,28 +51,31 @@ func (c *spoc) checkSubnetOf() {
 }
 
 func (c *spoc) checkIPAddressesAndBridges() {
-	prefix2net := make(map[string][]*network)
+	type nameV46 struct {
+		name string
+		v6   bool
+	}
+	prefix2net := make(map[nameV46][]*network)
 	for _, n := range c.allNetworks {
-		// Group bridged networks by prefix of name.
+		// Group bridged networks by IPv4/v6 type and prefix of name.
 		if n.ipType == bridgedIP {
 			prefix, _, _ := strings.Cut(n.name, "/")
-			prefix2net[prefix] = append(prefix2net[prefix], n)
+			prefixV46 := nameV46{prefix, n.ipV6}
+			prefix2net[prefixV46] = append(prefix2net[prefixV46], n)
 		} else if n.ipType == unnumberedIP {
 			l := n.interfaces
 			if len(l) > 2 {
 				c.err(
 					"Unnumbered %s is connected to more than two interfaces:\n%s",
-					n.name, l.nameList())
+					n.vxName(), l.nameList())
 			}
 		} else if !(n.ipType == tunnelIP || n.loopback) {
 			c.checkIPAddr(n)
 		}
 	}
 
 	// Check address conflicts for collected parts of bridged networks.
-	// Sort prefix names for deterministic error messages.
-	for _, prefix := range slices.Sorted(maps.Keys(prefix2net)) {
-		l := prefix2net[prefix]
+	for prefixV46, l := range prefix2net {
 		dummy := new(network)
 		seen := make(map[*routerIntf]bool)
 		for _, n := range l {
@@ -91,7 +92,7 @@ func (c *spoc) checkIPAddressesAndBridges() {
 		c.checkIPAddr(dummy)
 
 		// Check collected parts of bridged networks.
-		c.checkBridgedNetworks(prefix, l)
+		c.checkBridgedNetworks(prefixV46.name, l)
 	}
 }
 
@@ -126,7 +127,8 @@ func (c *spoc) checkIPAddr(n *network) {
 			ip := intf.ip
 			if other, found := ip2name[ip]; found {
 				if !(intf.redundant && redundant[other]) {
-					c.err("Duplicate IP address for %s and %s", other, intf)
+					c.err("Duplicate IP address for %s and %s",
+						other, intf.vxName())
 				}
 			} else {
 				ip2name[ip] = intf.name
@@ -139,7 +141,7 @@ func (c *spoc) checkIPAddr(n *network) {
 	if shortIntf != nil && routeIntf != nil {
 		c.err("Can't generate static routes for %s"+
 			" because IP address is unknown for:\n%s",
-			routeIntf, shortIntf.nameList())
+			routeIntf.vxName(), shortIntf.nameList())
 	}
 
 	// Optimization: No need to collect .routeInZone at bridge router.
@@ -156,7 +158,7 @@ func (c *spoc) checkIPAddr(n *network) {
 		}
 		rg := h.ipRange
 		if other, found := range2name[rg]; found {
-			c.err("Duplicate IP address for %s and %s", other, h)
+			c.err("Duplicate IP address for %s and %s", other, h.vxName())
 		} else {
 			range2name[rg] = h.name
 		}
@@ -170,15 +172,15 @@ func (c *spoc) checkIPAddr(n *network) {
 		}
 		for ip, other := range ip2name {
 			if rg.Contains(ip) {
-				c.err("Duplicate IP address for %s and %s", other, h)
+				c.err("Duplicate IP address for %s and %s", other, h.vxName())
 			}
 		}
 	}
 
 	for _, h := range n.hosts {
 		if h.ip.IsValid() {
 			if other, found := ip2name[h.ip]; found {
-				c.err("Duplicate IP address for %s and %s", other, h)
+				c.err("Duplicate IP address for %s and %s", other, h.vxName())
 			} else {
 				ip2name[h.ip] = h.name
 			}
@@ -198,12 +200,12 @@ func (c *spoc) checkBridgedNetworks(prefix string, l netList) {
 	if n, found := c.symTable.network[prefix[len("network:"):]]; found {
 		c.err(
 			"Must not define %s together with bridged networks of same name",
-			n)
+			n.vxName())
 	}
 	n1 := l[0]
 	group := l[1:]
 	if len(group) == 0 {
-		c.warn("Bridged %s must not be used solitary", n1)
+		c.warn("Bridged %s must not be used solitary", n1.vxName())
 	}
 	seen := make(map[*router]bool)
 	connected := make(map[*network]bool)
@@ -214,7 +216,8 @@ func (c *spoc) checkBridgedNetworks(prefix string, l netList) {
 		n2 := next[0]
 		next = next[1:]
 		if n1.ipp != n2.ipp {
-			c.err("%s and %s must have identical address", n1, n2)
+			c.err("%s and %s must have identical address",
+				n1.vxName(), n2.vxName())
 		}
 		connected[n2] = true
 		for _, in := range n2.interfaces {
@@ -230,7 +233,7 @@ func (c *spoc) checkBridgedNetworks(prefix string, l netList) {
 			if l3 := in.layer3Intf; l3 != nil {
 				if !n1.ipp.Contains(l3.ip) {
 					c.err("%s's IP doesn't match address of bridged networks",
-						l3)
+						l3.vxName())
 				}
 			}
 			for _, out := range r.interfaces {
@@ -246,7 +249,7 @@ func (c *spoc) checkBridgedNetworks(prefix string, l netList) {
 	}
 	for _, n2 := range group {
 		if !connected[n2] {
-			c.err("%s and %s must be connected by bridge", n2, n1)
+			c.err("%s and %s must be connected by bridge", n2.vxName(), n1.vxName())
 		}
 	}
 }

go/pkg/pass1/find-subnets.go

@@ -122,7 +122,7 @@ func (c *spoc) findSubnetsInZoneCluster0(z0 *zone) {
 		// Found two different networks with identical address.
 		if other := ipMap[ipp.Addr()]; other != nil {
 			c.err("%s and %s have identical address in %s",
-				other.name, n.name, z0.name)
+				other.vxName(), n.vxName(), z0.name)
 		} else {
 
 			// Store original network under its address.
@@ -422,7 +422,7 @@ func (c *spoc) findSubnetsInNatDomain0(domains []*natDomain, networks netList) {
 						for _, z := range domain.zones {
 							if z.cluster[0] == cl {
 								c.err("%s and %s have identical address in %s",
-									n, other, z)
+									n.vxName(), other.vxName(), z)
 								break
 							}
 						}

go/pkg/pass1/setup-objects.go

@@ -431,15 +431,6 @@ func (c *spoc) getAndCheckLayer3(r *router, a *ast.Router) string {
 				bName = a1.Name
 			}
 		}
-		if l3Name != "" {
-			// Check existence of layer3 interface.
-			if !slices.ContainsFunc(a.Interfaces, func(a1 *ast.Attribute) bool {
-				return a1.Name == l3Name
-			}) {
-				c.err("Must define %s at %s for corresponding bridge interfaces",
-					l3Name, a.Name)
-			}
-		}
 	}
 	return l3Name
 }
@@ -1820,6 +1811,7 @@ func (c *spoc) setupRouter2(r *router) {
 		}
 
 		isCryptoHub := false
+		var layer3Intf *routerIntf
 		for _, intf := range r.interfaces {
 			if intf.hub != nil || intf.spoke != nil {
 				if r.model.crypto == "" {
@@ -1833,9 +1825,24 @@ func (c *spoc) setupRouter2(r *router) {
 			// Link bridged interfaces with corresponding layer3 device.
 			// Used in findAutoInterfaces.
 			if intf.ipType == bridgedIP {
-				layer3Name := intf.name[len("interface:"):]
-				layer3Name, _, _ = strings.Cut(layer3Name, "/")
-				intf.layer3Intf = c.symTable.routerIntf[layer3Name]
+				if layer3Intf == nil {
+					i := slices.IndexFunc(r.interfaces, func(intf *routerIntf) bool {
+						if intf.isLayer3 {
+							return true
+						}
+						return false
+					})
+					if i == -1 {
+						l3Name, _, _ := strings.Cut(intf.vxName(), "/")
+						c.err("Must define %s for corresponding bridge interfaces",
+							l3Name)
+						// Prevent further errors.
+						layer3Intf = intf
+					} else {
+						layer3Intf = r.interfaces[i]
+					}
+				}
+				intf.layer3Intf = layer3Intf
 			}
 		}
 		if r.model.doAuth {

go/pkg/pass1/types.go

@@ -184,6 +184,19 @@ func (x *network) getUp() someObj {
 }
 func (x *network) intfList() intfList { return x.interfaces }
 func (x network) isCombined46() bool  { return x.combined46 != nil }
+func (x network) vxName() string {
+	return vxName(x.name, x.ipV6, x.combined46 != nil)
+}
+
+func vxName(name string, ipV6, isCombined46 bool) string {
+	if isCombined46 {
+		if ipV6 {
+			return "IPv6 " + name
+		}
+		return "IPv4 " + name
+	}
+	return name
+}
 
 type netList []*network
 
@@ -225,6 +238,9 @@ type host struct {
 }
 
 func (x host) isCombined46() bool { return x.combined46 != nil }
+func (x host) vxName() string {
+	return vxName(x.name, x.ipV6, x.combined46 != nil)
+}
 
 type model struct {
 	commentChar            string
@@ -388,6 +404,9 @@ func (intf routerIntf) getCrypto() *crypto {
 	return intf.realIntf.spoke
 }
 func (x routerIntf) isCombined46() bool { return x.combined46 != nil }
+func (x routerIntf) vxName() string {
+	return vxName(x.name, x.ipV6, x.combined46 != nil)
+}
 
 type intfList []*routerIntf
 
@@ -536,13 +555,7 @@ type area struct {
 
 func (x area) String() string { return x.name }
 func (x area) vxName() string {
-	if x.combined46 != nil {
-		if x.ipV6 {
-			return "IPv6 " + x.name
-		}
-		return "IPv4 " + x.name
-	}
-	return x.name
+	return vxName(x.name, x.ipV6, x.combined46 != nil)
 }
 func (x area) isCombined46() bool { return x.combined46 != nil }
 

go/testdata/bridged.t

@@ -191,7 +191,7 @@ router:bridge = {
 }
 network:n1/right = { ip = 10.1.1.0/24; }
 =ERROR=
-Error: Must define interface:n1 at router:bridge for corresponding bridge interfaces
+Error: Must define interface:bridge.n1 for corresponding bridge interfaces
 =END=
 
 ############################################################

go/testdata/ipv46-combined.t

@@ -901,3 +901,79 @@ network:n1 = { ip = 10.1.1.0/24; ip6 = 2001:db8:1:1::/64; }
 Error: Duplicate any:n1-v4 and any:n1-v6 in any:[network:n1-v6]
 Error: Duplicate any:n1-v4 and any:n1-v6 in any:[network:n1-v4]
 =END=
+
+############################################################
+=TITLE=Bridged network
+=INPUT=
+network:n1/left = {
+ ip = 10.1.1.0/24; ip6 = 2001:db8:1:1::/64;
+}
+router:bridge = {
+ model = ASA;
+ managed;
+ interface:n1 = { ip = 10.1.1.9; ip6 = 2001:db8:1:1::9; hardware = device; }
+ interface:n1/left  = { hardware = left; }
+ interface:n1/right = { hardware = right; }
+}
+network:n1/right = {
+ ip = 10.1.1.0/24; ip6 = 2001:db8:1:1::/64;
+}
+=WARNING=NONE
+
+############################################################
+=TITLE=Bridge with missing IPv6 address
+=INPUT=
+network:n1/left = {
+ ip = 10.1.1.0/24; ip6 = 2001:db8:1:1::/64;
+}
+router:bridge = {
+ model = ASA;
+ managed;
+ interface:n1 = { ip = 10.1.1.9; hardware = device; }
+ interface:n1/left  = { hardware = left; }
+ interface:n1/right = { hardware = right; }
+}
+network:n1/right = {
+ ip = 10.1.1.0/24; ip6 = 2001:db8:1:1::/64;
+}
+=ERROR=
+Error: Must define IPv6 interface:bridge.n1 for corresponding bridge interfaces
+=END=
+
+############################################################
+=TITLE=IPv6 part of unnumbered network has more than two interfaces
+=INPUT=
+network:u = { unnumbered; unnumbered6; }
+router:r1 = { interface:u = { unnumbered6; } }
+router:r2 = { interface:u = { unnumbered; unnumbered6; } }
+router:r3 = { interface:u = { unnumbered; unnumbered6; } }
+=ERROR=
+Error: Unnumbered IPv6 network:u is connected to more than two interfaces:
+ - interface:r1.u
+ - interface:r2.u
+ - interface:r3.u
+=END=
+
+############################################################
+=TITLE=Duplicate IPv6 address of hosts
+=INPUT=
+network:n1 = { ip = 10.1.1.0/24; ip6 = 2001:db8:1:1::/64;
+ host:h1 = { ip = 10.1.1.1; ip6 = 2001:db8:1:1::1; }
+ host:h2 = { ip = 10.1.1.2; ip6 = 2001:db8:1:1::1; }
+}
+=ERROR=
+Error: Duplicate IP address for host:h1 and IPv6 host:h2
+=END=
+
+############################################################
+=TITLE=Duplicate IPv4 address of networks
+=INPUT=
+network:n1 = { ip = 10.1.1.0/24; ip6 = 2001:db8:1:1::/64; }
+network:n2 = { ip = 10.1.1.0/24; ip6 = 2001:db8:1:2::/64; }
+router:r1 = {
+ interface:n1;
+ interface:n2;
+}
+=ERROR=
+Error: IPv4 network:n1 and IPv4 network:n2 have identical address in any:[network:n1]
+=END=

go/testdata/ipv6/bridged_ipv6.t

@@ -202,7 +202,7 @@ router:bridge = {
 }
 network:n1/right = { ip = ::a01:100/120; }
 =ERROR=
-Error: Must define interface:n1 at router:bridge for corresponding bridge interfaces
+Error: Must define interface:bridge.n1 for corresponding bridge interfaces
 =END=
 
 ############################################################