Merge pull request #16866 from danwinship/egress-ip-fixes

Automatic merge from submit-queue. Egress IP fixes 1. Further IP address validation, from #16779 (comment) 2. Fix OVS VXLAN ingress rule to not filter out remote node egress IP traffic, fixing https://bugzilla.redhat.com/show_bug.cgi?id=1501876
openshift · Oct 20, 2017 · 747a173 · 747a173
2 parents e99d0e9 + 64b1dc7
commit 747a173
Show file tree

Hide file tree

Showing 2 changed files with 37 additions and 33 deletions.
diff --git a/pkg/network/node/egressip.go b/pkg/network/node/egressip.go
@@ -52,6 +52,7 @@ type egressIPWatcher struct {
 	namespacesByEgressIP map[string]*namespaceEgress
 
 	localEgressLink      netlink.Link
+	localEgressNet       *net.IPNet
 	localEgressIPMaskLen int
 
 	testModeChan chan string
@@ -71,6 +72,10 @@ func newEgressIPWatcher(localIP string, oc *ovsController) *egressIPWatcher {
 }
 
 func (eip *egressIPWatcher) Start(networkClient networkclient.Interface, iptables *NodeIPTables) error {
+	if err := eip.findEgressLink(); err != nil {
+		return fmt.Errorf("could not find egress network interface: %v", err)
+	}
+
 	eip.iptables = iptables
 	eip.networkClient = networkClient
 
@@ -79,6 +84,34 @@ func (eip *egressIPWatcher) Start(networkClient networkclient.Interface, iptable
 	return nil
 }
 
+func (eip *egressIPWatcher) findEgressLink() error {
+	links, err := netlink.LinkList()
+	if err != nil {
+		return err
+	}
+	for _, link := range links {
+		addrs, err := netlink.AddrList(link, syscall.AF_INET)
+		if err != nil {
+			glog.Warningf("Could not get addresses of interface %q while trying to find egress interface: %v", link.Attrs().Name, err)
+			continue
+		}
+
+		for _, addr := range addrs {
+			if addr.IP.String() == eip.localIP {
+				_, eip.localEgressNet, err = net.ParseCIDR(addr.IPNet.String())
+				if err != nil {
+					return fmt.Errorf("could not parse CIDR network from address %q: %v", addr.IP.String(), err)
+				}
+				eip.localEgressLink = link
+				eip.localEgressIPMaskLen, _ = addr.Mask.Size()
+				return nil
+			}
+		}
+	}
+
+	return fmt.Errorf("could not find network interface with the address %q", eip.localIP)
+}
+
 func ipToHex(ip string) string {
 	bytes := net.ParseIP(ip)
 	if bytes == nil {
@@ -254,38 +287,14 @@ func (eip *egressIPWatcher) claimEgressIP(egressIP, egressHex string) error {
 		return nil
 	}
 
-	if eip.localEgressLink == nil {
-		links, err := netlink.LinkList()
-		if err != nil {
-			return fmt.Errorf("could not get list of network interfaces while adding egress IP: %v", err)
-		}
-	linkLoop:
-		for _, link := range links {
-			addrs, err := netlink.AddrList(link, syscall.AF_INET)
-			if err != nil {
-				glog.Warningf("Could not get addresses of interface %q while trying to find egress interface: %v", link.Attrs().Name, err)
-				continue
-			}
-
-			for _, addr := range addrs {
-				if addr.IP.String() == eip.localIP {
-					eip.localEgressLink = link
-					eip.localEgressIPMaskLen, _ = addr.Mask.Size()
-					break linkLoop
-				}
-			}
-		}
-
-		if eip.localEgressLink == nil {
-			return fmt.Errorf("could not find network interface with the address %q while adding egress IP", eip.localIP)
-		}
-	}
-
 	egressIPNet := fmt.Sprintf("%s/%d", egressIP, eip.localEgressIPMaskLen)
 	addr, err := netlink.ParseAddr(egressIPNet)
 	if err != nil {
 		return fmt.Errorf("could not parse egress IP %q: %v", egressIPNet, err)
 	}
+	if !eip.localEgressNet.Contains(addr.IP) {
+		return fmt.Errorf("egress IP %q is not in local network %s of interface %s", egressIP, eip.localEgressNet.String(), eip.localEgressLink.Attrs().Name)
+	}
 	err = netlink.AddrAdd(eip.localEgressLink, addr)
 	if err != nil {
 		return fmt.Errorf("could not add egress IP %q to %s: %v", egressIPNet, eip.localEgressLink.Attrs().Name, err)
@@ -308,10 +317,6 @@ func (eip *egressIPWatcher) releaseEgressIP(egressIP, egressHex string) error {
 		return nil
 	}
 
-	if eip.localEgressLink == nil {
-		return nil
-	}
-
 	egressIPNet := fmt.Sprintf("%s/%d", egressIP, eip.localEgressIPMaskLen)
 	addr, err := netlink.ParseAddr(egressIPNet)
 	if err != nil {

diff --git a/pkg/network/node/ovscontroller.go b/pkg/network/node/ovscontroller.go
@@ -93,8 +93,7 @@ func (oc *ovsController) SetupOVS(clusterNetworkCIDR []string, serviceNetworkCID
 	// vxlan0
 	for _, clusterCIDR := range clusterNetworkCIDR {
 		otx.AddFlow("table=0, priority=200, in_port=1, arp, nw_src=%s, nw_dst=%s, actions=move:NXM_NX_TUN_ID[0..31]->NXM_NX_REG0[],goto_table:10", clusterCIDR, localSubnetCIDR)
-		otx.AddFlow("table=0, priority=200, in_port=1, ip, nw_src=%s, nw_dst=%s, actions=move:NXM_NX_TUN_ID[0..31]->NXM_NX_REG0[],goto_table:10", clusterCIDR, localSubnetCIDR)
-		otx.AddFlow("table=0, priority=200, in_port=1, ip, nw_src=%s, nw_dst=224.0.0.0/4, actions=move:NXM_NX_TUN_ID[0..31]->NXM_NX_REG0[],goto_table:10", clusterCIDR)
+		otx.AddFlow("table=0, priority=200, in_port=1, ip, nw_src=%s, actions=move:NXM_NX_TUN_ID[0..31]->NXM_NX_REG0[],goto_table:10", clusterCIDR)
 	}
 	otx.AddFlow("table=0, priority=150, in_port=1, actions=drop")
 	// tun0