Fix for SDN-IP installing wrong point-to-point intents.
Fixes ONOS-2514.
Ported from onos-1.2 branch.
Change-Id: I0d3d6012daa8dd2a45707a58cf6e745314f6dc24
(cherry picked from commit 0c763e9b064eac86d866b8fdfea25baec104324e)
diff --git a/apps/routing-api/src/main/java/org/onosproject/routing/config/RoutingConfigurationService.java b/apps/routing-api/src/main/java/org/onosproject/routing/config/RoutingConfigurationService.java
index 056a195..bbd9560 100644
--- a/apps/routing-api/src/main/java/org/onosproject/routing/config/RoutingConfigurationService.java
+++ b/apps/routing-api/src/main/java/org/onosproject/routing/config/RoutingConfigurationService.java
@@ -98,6 +98,14 @@
Interface getInterface(ConnectPoint connectPoint);
/**
+ * Retrieves the interface associated with the given IP address.
+ *
+ * @param ip IP address of the interface
+ * @return the interface
+ */
+ Interface getInterface(IpAddress ip);
+
+ /**
* Retrieves the interface that matches the given IP address. Matching
* means that the IP address is in one of the interface's assigned subnets.
*
diff --git a/apps/routing/src/main/java/org/onosproject/routing/config/impl/HostToInterfaceAdaptor.java b/apps/routing/src/main/java/org/onosproject/routing/config/impl/HostToInterfaceAdaptor.java
index 8fabe39..78eecb8 100644
--- a/apps/routing/src/main/java/org/onosproject/routing/config/impl/HostToInterfaceAdaptor.java
+++ b/apps/routing/src/main/java/org/onosproject/routing/config/impl/HostToInterfaceAdaptor.java
@@ -63,6 +63,20 @@
return null;
}
+ public Interface getInterface(IpAddress ip) {
+ Set<PortAddresses> portAddresses = hostService.getAddressBindings();
+
+ for (PortAddresses portAddress : portAddresses) {
+ for (InterfaceIpAddress portIp : portAddress.ipAddresses()) {
+ if (portIp.ipAddress().equals(ip)) {
+ return new Interface(portAddress);
+ }
+ }
+ }
+
+ return null;
+ }
+
public Interface getMatchingInterface(IpAddress ipAddress) {
checkNotNull(ipAddress);
diff --git a/apps/routing/src/main/java/org/onosproject/routing/config/impl/RoutingConfigurationImpl.java b/apps/routing/src/main/java/org/onosproject/routing/config/impl/RoutingConfigurationImpl.java
index 2667594..e45548f 100644
--- a/apps/routing/src/main/java/org/onosproject/routing/config/impl/RoutingConfigurationImpl.java
+++ b/apps/routing/src/main/java/org/onosproject/routing/config/impl/RoutingConfigurationImpl.java
@@ -187,6 +187,11 @@
}
@Override
+ public Interface getInterface(IpAddress ip) {
+ return hostAdaptor.getInterface(ip);
+ }
+
+ @Override
public Interface getMatchingInterface(IpAddress ipAddress) {
return hostAdaptor.getMatchingInterface(ipAddress);
}
diff --git a/apps/routing/src/test/java/org/onosproject/routing/config/impl/HostToInterfaceAdaptorTest.java b/apps/routing/src/test/java/org/onosproject/routing/config/impl/HostToInterfaceAdaptorTest.java
index 5112427..0347fc5 100644
--- a/apps/routing/src/test/java/org/onosproject/routing/config/impl/HostToInterfaceAdaptorTest.java
+++ b/apps/routing/src/test/java/org/onosproject/routing/config/impl/HostToInterfaceAdaptorTest.java
@@ -173,7 +173,8 @@
*/
@Test(expected = NullPointerException.class)
public void testGetInterfaceNull() {
- adaptor.getInterface(null);
+ ConnectPoint c = null;
+ adaptor.getInterface(c);
}
/**
diff --git a/apps/sdnip/src/main/java/org/onosproject/sdnip/PeerConnectivityManager.java b/apps/sdnip/src/main/java/org/onosproject/sdnip/PeerConnectivityManager.java
index f9ee791..0bb18ed 100644
--- a/apps/sdnip/src/main/java/org/onosproject/sdnip/PeerConnectivityManager.java
+++ b/apps/sdnip/src/main/java/org/onosproject/sdnip/PeerConnectivityManager.java
@@ -139,24 +139,22 @@
List<InterfaceAddress> interfaceAddresses =
bgpSpeaker.interfaceAddresses();
- Interface peerInterface = configService.getInterface(
- bgpPeer.connectPoint());
-
- if (peerInterface == null) {
- log.error("No interface found for peer {}", bgpPeer.ipAddress());
- return intents;
- }
-
IpAddress bgpdAddress = null;
for (InterfaceAddress interfaceAddress : interfaceAddresses) {
- if (interfaceAddress.connectPoint().equals(peerInterface.connectPoint())) {
- for (InterfaceIpAddress interfaceIpAddress : peerInterface.ipAddresses()) {
- // Only add intents where the peer and ONOS's addresses are
- // in the same subnet
- if (interfaceIpAddress.subnetAddress().contains(bgpPeer.ipAddress())) {
- bgpdAddress = interfaceAddress.ipAddress();
- }
+ Interface peerInterface = configService.getInterface(interfaceAddress.ipAddress());
+ if (peerInterface == null) {
+ continue;
+ }
+
+ for (InterfaceIpAddress interfaceIpAddress : peerInterface.ipAddresses()) {
+ // Only add intents where the peer and ONOS's addresses are
+ // in the same subnet
+ if (interfaceIpAddress.subnetAddress().contains(bgpPeer.ipAddress())) {
+ bgpdAddress = interfaceAddress.ipAddress();
+ break;
}
+ }
+ if (bgpdAddress != null) {
break;
}
}
@@ -167,7 +165,7 @@
}
IpAddress bgpdPeerAddress = bgpPeer.ipAddress();
- ConnectPoint bgpdPeerConnectPoint = peerInterface.connectPoint();
+ ConnectPoint bgpdPeerConnectPoint = bgpPeer.connectPoint();
if (bgpdAddress.version() != bgpdPeerAddress.version()) {
return intents;
diff --git a/apps/sdnip/src/test/java/org/onosproject/sdnip/PeerConnectivityManagerTest.java b/apps/sdnip/src/test/java/org/onosproject/sdnip/PeerConnectivityManagerTest.java
index 523630d..fd7bb46 100644
--- a/apps/sdnip/src/test/java/org/onosproject/sdnip/PeerConnectivityManagerTest.java
+++ b/apps/sdnip/src/test/java/org/onosproject/sdnip/PeerConnectivityManagerTest.java
@@ -15,13 +15,7 @@
*/
package org.onosproject.sdnip;
-import java.util.ArrayList;
-import java.util.Collections;
-import java.util.HashMap;
-import java.util.LinkedList;
-import java.util.List;
-import java.util.Map;
-
+import com.google.common.collect.Sets;
import org.junit.Before;
import org.junit.Ignore;
import org.junit.Test;
@@ -52,7 +46,12 @@
import org.onosproject.routing.config.InterfaceAddress;
import org.onosproject.routing.config.RoutingConfigurationService;
-import com.google.common.collect.Sets;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.Map;
import static org.easymock.EasyMock.createMock;
import static org.easymock.EasyMock.expect;
@@ -144,7 +143,6 @@
"00:00:00:00:00:01");
List<InterfaceAddress> interfaceAddresses1 = new LinkedList<>();
interfaceAddresses1.add(new InterfaceAddress(dpid1, 1, "192.168.10.101"));
- interfaceAddresses1.add(new InterfaceAddress(dpid2, 1, "192.168.20.101"));
bgpSpeaker1.setInterfaceAddresses(interfaceAddresses1);
configuredBgpSpeakers.put(bgpSpeaker1.name(), bgpSpeaker1);
@@ -154,21 +152,11 @@
"00:00:00:00:00:00:00:01", 100,
"00:00:00:00:00:02");
List<InterfaceAddress> interfaceAddresses2 = new LinkedList<>();
- interfaceAddresses2.add(new InterfaceAddress(dpid1, 1, "192.168.10.102"));
- interfaceAddresses2.add(new InterfaceAddress(dpid2, 1, "192.168.20.102"));
+ interfaceAddresses2.add(new InterfaceAddress(dpid2, 1, "192.168.20.101"));
+ interfaceAddresses2.add(new InterfaceAddress(dpid2, 1, "192.168.30.101"));
bgpSpeaker2.setInterfaceAddresses(interfaceAddresses2);
configuredBgpSpeakers.put(bgpSpeaker2.name(), bgpSpeaker2);
- BgpSpeaker bgpSpeaker3 = new BgpSpeaker(
- "bgpSpeaker3",
- "00:00:00:00:00:00:00:02", 100,
- "00:00:00:00:00:03");
- List<InterfaceAddress> interfaceAddresses3 = new LinkedList<>();
- interfaceAddresses3.add(new InterfaceAddress(dpid1, 1, "192.168.10.103"));
- interfaceAddresses3.add(new InterfaceAddress(dpid2, 1, "192.168.20.103"));
- bgpSpeaker3.setInterfaceAddresses(interfaceAddresses3);
- configuredBgpSpeakers.put(bgpSpeaker3.name(), bgpSpeaker3);
-
return configuredBgpSpeakers;
}
@@ -184,7 +172,7 @@
String interfaceSw1Eth1 = "s1-eth1";
InterfaceIpAddress ia1 =
- new InterfaceIpAddress(IpAddress.valueOf("192.168.10.1"),
+ new InterfaceIpAddress(IpAddress.valueOf("192.168.10.101"),
IpPrefix.valueOf("192.168.10.0/24"));
Interface intfsw1eth1 = new Interface(s1Eth1,
Collections.singleton(ia1),
@@ -194,7 +182,7 @@
configuredInterfaces.put(interfaceSw1Eth1, intfsw1eth1);
String interfaceSw2Eth1 = "s2-eth1";
InterfaceIpAddress ia2 =
- new InterfaceIpAddress(IpAddress.valueOf("192.168.20.2"),
+ new InterfaceIpAddress(IpAddress.valueOf("192.168.20.101"),
IpPrefix.valueOf("192.168.20.0/24"));
Interface intfsw2eth1 = new Interface(s2Eth1,
Collections.singleton(ia2),
@@ -202,10 +190,28 @@
VlanId.NONE);
configuredInterfaces.put(interfaceSw2Eth1, intfsw2eth1);
+ String interfaceSw2Eth1intf2 = "s2-eth1_2";
+ InterfaceIpAddress ia3 =
+ new InterfaceIpAddress(IpAddress.valueOf("192.168.30.101"),
+ IpPrefix.valueOf("192.168.30.0/24"));
+ Interface intfsw2eth1intf2 = new Interface(s2Eth1,
+ Collections.singleton(ia3),
+ MacAddress.valueOf("00:00:00:00:00:03"),
+ VlanId.NONE);
+ configuredInterfaces.put(interfaceSw2Eth1intf2, intfsw2eth1intf2);
+
expect(routingConfig.getInterface(s1Eth1))
.andReturn(intfsw1eth1).anyTimes();
+ expect(routingConfig.getInterface(IpAddress.valueOf("192.168.10.101")))
+ .andReturn(intfsw1eth1).anyTimes();
expect(routingConfig.getInterface(s2Eth1))
.andReturn(intfsw2eth1).anyTimes();
+ expect(routingConfig.getInterface(IpAddress.valueOf("192.168.20.101")))
+ .andReturn(intfsw2eth1).anyTimes();
+ //expect(routingConfig.getInterface(s2Eth1))
+ // .andReturn(intfsw2eth1intf2).anyTimes();
+ expect(routingConfig.getInterface(IpAddress.valueOf("192.168.30.101")))
+ .andReturn(intfsw2eth1intf2).anyTimes();
// Non-existent interface used during one of the tests
expect(routingConfig.getInterface(new ConnectPoint(
@@ -237,7 +243,7 @@
configuredPeers.put(IpAddress.valueOf(peer1Sw2Eth1),
new BgpPeer(dpid2, 1, peer1Sw2Eth1));
- String peer2Sw2Eth1 = "192.168.20.2";
+ String peer2Sw2Eth1 = "192.168.30.1";
configuredPeers.put(IpAddress.valueOf(peer2Sw2Eth1),
new BgpPeer(dpid2, 1, peer2Sw2Eth1));
@@ -338,111 +344,21 @@
"192.168.20.1/32", "192.168.20.101/32", bgpPort, null,
s2Eth1, s1Eth100);
- // Start to build intents between BGP speaker1 and BGP peer3
- bgpPathintentConstructor(
- "192.168.20.101/32", "192.168.20.2/32", null, bgpPort,
- s1Eth100, s2Eth1);
- bgpPathintentConstructor(
- "192.168.20.101/32", "192.168.20.2/32", bgpPort, null,
- s1Eth100, s2Eth1);
-
- bgpPathintentConstructor(
- "192.168.20.2/32", "192.168.20.101/32", null, bgpPort,
- s2Eth1, s1Eth100);
- bgpPathintentConstructor(
- "192.168.20.2/32", "192.168.20.101/32", bgpPort, null,
- s2Eth1, s1Eth100);
-
- //
- // Start to build intents between BGP speaker2 and BGP peer1
- bgpPathintentConstructor(
- "192.168.10.102/32", "192.168.10.1/32", null, bgpPort,
- s1Eth100, s1Eth1);
- bgpPathintentConstructor(
- "192.168.10.102/32", "192.168.10.1/32", bgpPort, null,
- s1Eth100, s1Eth1);
-
- bgpPathintentConstructor(
- "192.168.10.1/32", "192.168.10.102/32", null, bgpPort,
- s1Eth1, s1Eth100);
- bgpPathintentConstructor(
- "192.168.10.1/32", "192.168.10.102/32", bgpPort, null,
- s1Eth1, s1Eth100);
- // Start to build intents between BGP speaker2 and BGP peer2
- bgpPathintentConstructor(
- "192.168.20.102/32", "192.168.20.1/32", null, bgpPort,
- s1Eth100, s2Eth1);
- bgpPathintentConstructor(
- "192.168.20.102/32", "192.168.20.1/32", bgpPort, null,
- s1Eth100, s2Eth1);
-
- bgpPathintentConstructor(
- "192.168.20.1/32", "192.168.20.102/32", null, bgpPort,
- s2Eth1, s1Eth100);
- bgpPathintentConstructor(
- "192.168.20.1/32", "192.168.20.102/32", bgpPort, null,
- s2Eth1, s1Eth100);
-
- // Start to build intents between BGP speaker2 and BGP peer3
- bgpPathintentConstructor(
- "192.168.20.102/32", "192.168.20.2/32", null, bgpPort,
- s1Eth100, s2Eth1);
- bgpPathintentConstructor(
- "192.168.20.102/32", "192.168.20.2/32", bgpPort, null,
- s1Eth100, s2Eth1);
-
- bgpPathintentConstructor(
- "192.168.20.2/32", "192.168.20.102/32", null, bgpPort,
- s2Eth1, s1Eth100);
- bgpPathintentConstructor(
- "192.168.20.2/32", "192.168.20.102/32", bgpPort, null,
- s2Eth1, s1Eth100);
-
//
// Start to build intents between BGP speaker3 and BGP peer1
bgpPathintentConstructor(
- "192.168.10.103/32", "192.168.10.1/32", null, bgpPort,
- s2Eth100, s1Eth1);
+ "192.168.30.101/32", "192.168.30.1/32", null, bgpPort,
+ s1Eth100, s2Eth1);
bgpPathintentConstructor(
- "192.168.10.103/32", "192.168.10.1/32", bgpPort, null,
- s2Eth100, s1Eth1);
+ "192.168.30.101/32", "192.168.30.1/32", bgpPort, null,
+ s1Eth100, s2Eth1);
bgpPathintentConstructor(
- "192.168.10.1/32", "192.168.10.103/32", null, bgpPort,
- s1Eth1, s2Eth100);
+ "192.168.30.1/32", "192.168.30.101/32", null, bgpPort,
+ s2Eth1, s1Eth100);
bgpPathintentConstructor(
- "192.168.10.1/32", "192.168.10.103/32", bgpPort, null,
- s1Eth1, s2Eth100);
-
- // Start to build intents between BGP speaker3 and BGP peer2
- bgpPathintentConstructor(
- "192.168.20.103/32", "192.168.20.1/32", null, bgpPort,
- s2Eth100, s2Eth1);
- bgpPathintentConstructor(
- "192.168.20.103/32", "192.168.20.1/32", bgpPort, null,
- s2Eth100, s2Eth1);
-
- bgpPathintentConstructor(
- "192.168.20.1/32", "192.168.20.103/32", null, bgpPort,
- s2Eth1, s2Eth100);
- bgpPathintentConstructor(
- "192.168.20.1/32", "192.168.20.103/32", bgpPort, null,
- s2Eth1, s2Eth100);
-
- // Start to build intents between BGP speaker3 and BGP peer3
- bgpPathintentConstructor(
- "192.168.20.103/32", "192.168.20.2/32", null, bgpPort,
- s2Eth100, s2Eth1);
- bgpPathintentConstructor(
- "192.168.20.103/32", "192.168.20.2/32", bgpPort, null,
- s2Eth100, s2Eth1);
-
- bgpPathintentConstructor(
- "192.168.20.2/32", "192.168.20.103/32", null, bgpPort,
- s2Eth1, s2Eth100);
- bgpPathintentConstructor(
- "192.168.20.2/32", "192.168.20.103/32", bgpPort, null,
- s2Eth1, s2Eth100);
+ "192.168.30.1/32", "192.168.30.101/32", bgpPort, null,
+ s2Eth1, s1Eth100);
}
/**
@@ -494,49 +410,10 @@
icmpPathintentConstructor(
"192.168.20.1/32", "192.168.20.101/32", s2Eth1, s1Eth100);
- // Start to build intents between BGP speaker1 and BGP peer3
icmpPathintentConstructor(
- "192.168.20.101/32", "192.168.20.2/32", s1Eth100, s2Eth1);
+ "192.168.30.101/32", "192.168.30.1/32", s1Eth100, s2Eth1);
icmpPathintentConstructor(
- "192.168.20.2/32", "192.168.20.101/32", s2Eth1, s1Eth100);
-
- //
- // Start to build intents between BGP speaker2 and BGP peer1
- icmpPathintentConstructor(
- "192.168.10.102/32", "192.168.10.1/32", s1Eth100, s1Eth1);
- icmpPathintentConstructor(
- "192.168.10.1/32", "192.168.10.102/32", s1Eth1, s1Eth100);
-
- // Start to build intents between BGP speaker2 and BGP peer2
- icmpPathintentConstructor(
- "192.168.20.102/32", "192.168.20.1/32", s1Eth100, s2Eth1);
- icmpPathintentConstructor(
- "192.168.20.1/32", "192.168.20.102/32", s2Eth1, s1Eth100);
-
- // Start to build intents between BGP speaker2 and BGP peer3
- icmpPathintentConstructor(
- "192.168.20.102/32", "192.168.20.2/32", s1Eth100, s2Eth1);
- icmpPathintentConstructor(
- "192.168.20.2/32", "192.168.20.102/32", s2Eth1, s1Eth100);
-
- //
- // Start to build intents between BGP speaker3 and BGP peer1
- icmpPathintentConstructor(
- "192.168.10.103/32", "192.168.10.1/32", s2Eth100, s1Eth1);
- icmpPathintentConstructor(
- "192.168.10.1/32", "192.168.10.103/32", s1Eth1, s2Eth100);
-
- // Start to build intents between BGP speaker3 and BGP peer2
- icmpPathintentConstructor(
- "192.168.20.103/32", "192.168.20.1/32", s2Eth100, s2Eth1);
- icmpPathintentConstructor(
- "192.168.20.1/32", "192.168.20.103/32", s2Eth1, s2Eth100);
-
- // Start to build intents between BGP speaker3 and BGP peer3
- icmpPathintentConstructor(
- "192.168.20.103/32", "192.168.20.2/32", s2Eth100, s2Eth1);
- icmpPathintentConstructor(
- "192.168.20.2/32", "192.168.20.103/32", s2Eth1, s2Eth100);
+ "192.168.30.1/32", "192.168.30.101/32", s2Eth1, s1Eth100);
}
@@ -601,6 +478,12 @@
.andReturn(null).anyTimes();
expect(routingConfig.getInterface(s1Eth1))
.andReturn(null).anyTimes();
+ expect(routingConfig.getInterface(IpAddress.valueOf("192.168.10.101")))
+ .andReturn(null).anyTimes();
+ expect(routingConfig.getInterface(IpAddress.valueOf("192.168.20.101")))
+ .andReturn(null).anyTimes();
+ expect(routingConfig.getInterface(IpAddress.valueOf("192.168.30.101")))
+ .andReturn(null).anyTimes();
expect(routingConfig.getBgpPeers()).andReturn(peers).anyTimes();
expect(routingConfig.getBgpSpeakers()).andReturn(bgpSpeakers).anyTimes();