Only remove TMAC flow when it is the last port within the same VLAN if TMAC doesn't support in_port matching
- Address an edge case that was not considered in 16418
- Add some unit tests
Change-Id: Ie999850ce0101b528391d45989390924411817f9
diff --git a/apps/segmentrouting/app/src/test/java/org/onosproject/segmentrouting/RoutingRulePopulatorTest.java b/apps/segmentrouting/app/src/test/java/org/onosproject/segmentrouting/RoutingRulePopulatorTest.java
new file mode 100644
index 0000000..4f5f226
--- /dev/null
+++ b/apps/segmentrouting/app/src/test/java/org/onosproject/segmentrouting/RoutingRulePopulatorTest.java
@@ -0,0 +1,187 @@
+/*
+ * Copyright 2018-present Open Networking Foundation
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.onosproject.segmentrouting;
+
+import com.google.common.collect.Lists;
+import com.google.common.collect.Maps;
+import com.google.common.collect.Sets;
+import org.easymock.EasyMock;
+import org.junit.Before;
+import org.junit.Test;
+import org.onlab.packet.VlanId;
+import org.onosproject.net.ConnectPoint;
+import org.onosproject.net.DefaultDevice;
+import org.onosproject.net.DefaultPort;
+import org.onosproject.net.Device;
+import org.onosproject.net.DeviceId;
+import org.onosproject.net.Port;
+import org.onosproject.net.PortNumber;
+import org.onosproject.net.device.DeviceService;
+import org.onosproject.net.intf.Interface;
+import org.onosproject.net.intf.InterfaceService;
+import org.onosproject.net.provider.ProviderId;
+import org.onosproject.segmentrouting.config.DeviceConfiguration;
+
+import java.util.List;
+import java.util.Set;
+
+import static org.easymock.EasyMock.anyObject;
+import static org.easymock.EasyMock.expect;
+import static org.easymock.EasyMock.replay;
+import static org.junit.Assert.*;
+
+public class RoutingRulePopulatorTest {
+ private RoutingRulePopulator rrp;
+ private SegmentRoutingManager srManager;
+ private InterfaceService interfaceService;
+ private DeviceService deviceService;
+
+ private final DeviceId devId1 = DeviceId.deviceId("of:1");
+ private final Device dev1 = new DefaultDevice(ProviderId.NONE, devId1, Device.Type.SWITCH,
+ null, null, null, null, null);
+
+ private final PortNumber p1 = PortNumber.portNumber(1);
+ private final PortNumber p2 = PortNumber.portNumber(2);
+ private final PortNumber p3 = PortNumber.portNumber(3);
+ private final PortNumber p4 = PortNumber.portNumber(4);
+ private final PortNumber p5 = PortNumber.portNumber(5);
+
+ private final VlanId v10 = VlanId.vlanId((short) 10);
+ private final VlanId v20 = VlanId.vlanId((short) 20);
+ private final VlanId vInt = SegmentRoutingManager.INTERNAL_VLAN;
+
+ private final Interface u10 = new Interface(null, new ConnectPoint(devId1, p1),
+ null, null, null, v10, null, null);
+ private final Interface t10 = new Interface(null, new ConnectPoint(devId1, p2),
+ null, null, null, null, Sets.newHashSet(v10), null);
+ private final Interface t10n20 = new Interface(null, new ConnectPoint(devId1, p3),
+ null, null, null, null, Sets.newHashSet(v10), v20);
+
+ @Before
+ public void setUp() throws Exception {
+ Set<Interface> interfaces = Sets.newHashSet(u10, t10, t10n20);
+ interfaceService = new MockInterfaceService(interfaces);
+ deviceService = EasyMock.createMock(DeviceService.class);
+ srManager = new MockSegmentRoutingManager(Maps.newHashMap());
+ srManager.deviceConfiguration = EasyMock.createMock(DeviceConfiguration.class);
+ srManager.interfaceService = interfaceService;
+ srManager.deviceService = deviceService;
+ rrp = new RoutingRulePopulator(srManager);
+ }
+
+ // All ports are enabled
+ @Test
+ public void testNoMoreEnabledPortCase1() throws Exception {
+ Port port1 = new DefaultPort(dev1, p1, true);
+ Port port2 = new DefaultPort(dev1, p2, true);
+ Port port3 = new DefaultPort(dev1, p3, true);
+ Port port4 = new DefaultPort(dev1, p4, true);
+ Port port5 = new DefaultPort(dev1, p5, true);
+ List<Port> ports = Lists.newArrayList(port1, port2, port3, port4, port5);
+
+ expect(deviceService.getPorts(anyObject(DeviceId.class))).andReturn(ports).anyTimes();
+ replay(deviceService);
+ assertFalse(rrp.noMoreEnabledPort(devId1, v10));
+ assertFalse(rrp.noMoreEnabledPort(devId1, v20));
+ assertFalse(rrp.noMoreEnabledPort(devId1, vInt));
+ }
+
+ // Disable port 1
+ @Test
+ public void testNoMoreEnabledPortCase2() throws Exception {
+ Port port1 = new DefaultPort(dev1, p1, false);
+ Port port2 = new DefaultPort(dev1, p2, true);
+ Port port3 = new DefaultPort(dev1, p3, true);
+ Port port4 = new DefaultPort(dev1, p4, true);
+ Port port5 = new DefaultPort(dev1, p5, true);
+ List<Port> ports = Lists.newArrayList(port1, port2, port3, port4, port5);
+
+ expect(deviceService.getPorts(anyObject(DeviceId.class))).andReturn(ports).anyTimes();
+ replay(deviceService);
+ assertFalse(rrp.noMoreEnabledPort(devId1, v10));
+ assertFalse(rrp.noMoreEnabledPort(devId1, v20));
+ assertFalse(rrp.noMoreEnabledPort(devId1, vInt));
+ }
+
+ // Disable port 1 and 3
+ @Test
+ public void testNoMoreEnabledPortCase3() throws Exception {
+ Port port1 = new DefaultPort(dev1, p1, false);
+ Port port2 = new DefaultPort(dev1, p2, true);
+ Port port3 = new DefaultPort(dev1, p3, false);
+ Port port4 = new DefaultPort(dev1, p4, true);
+ Port port5 = new DefaultPort(dev1, p5, true);
+ List<Port> ports = Lists.newArrayList(port1, port2, port3, port4, port5);
+
+ expect(deviceService.getPorts(anyObject(DeviceId.class))).andReturn(ports).anyTimes();
+ replay(deviceService);
+ assertFalse(rrp.noMoreEnabledPort(devId1, v10));
+ assertTrue(rrp.noMoreEnabledPort(devId1, v20));
+ assertFalse(rrp.noMoreEnabledPort(devId1, vInt));
+ }
+
+ // Disable port 1 to 3
+ @Test
+ public void testNoMoreEnabledPortCase4() throws Exception {
+ Port port1 = new DefaultPort(dev1, p1, false);
+ Port port2 = new DefaultPort(dev1, p2, false);
+ Port port3 = new DefaultPort(dev1, p3, false);
+ Port port4 = new DefaultPort(dev1, p4, true);
+ Port port5 = new DefaultPort(dev1, p5, true);
+ List<Port> ports = Lists.newArrayList(port1, port2, port3, port4, port5);
+
+ expect(deviceService.getPorts(anyObject(DeviceId.class))).andReturn(ports).anyTimes();
+ replay(deviceService);
+ assertTrue(rrp.noMoreEnabledPort(devId1, v10));
+ assertTrue(rrp.noMoreEnabledPort(devId1, v20));
+ assertFalse(rrp.noMoreEnabledPort(devId1, vInt));
+ }
+
+ // Disable port 1 to 4
+ @Test
+ public void testNoMoreEnabledPortCase5() throws Exception {
+ Port port1 = new DefaultPort(dev1, p1, false);
+ Port port2 = new DefaultPort(dev1, p2, false);
+ Port port3 = new DefaultPort(dev1, p3, false);
+ Port port4 = new DefaultPort(dev1, p4, false);
+ Port port5 = new DefaultPort(dev1, p5, true);
+ List<Port> ports = Lists.newArrayList(port1, port2, port3, port4, port5);
+
+ expect(deviceService.getPorts(anyObject(DeviceId.class))).andReturn(ports).anyTimes();
+ replay(deviceService);
+ assertTrue(rrp.noMoreEnabledPort(devId1, v10));
+ assertTrue(rrp.noMoreEnabledPort(devId1, v20));
+ assertFalse(rrp.noMoreEnabledPort(devId1, vInt));
+ }
+
+ // Disable all ports
+ @Test
+ public void testNoMoreEnabledPortCase6() throws Exception {
+ Port port1 = new DefaultPort(dev1, p1, false);
+ Port port2 = new DefaultPort(dev1, p2, false);
+ Port port3 = new DefaultPort(dev1, p3, false);
+ Port port4 = new DefaultPort(dev1, p4, false);
+ Port port5 = new DefaultPort(dev1, p5, false);
+ List<Port> ports = Lists.newArrayList(port1, port2, port3, port4, port5);
+
+ expect(deviceService.getPorts(anyObject(DeviceId.class))).andReturn(ports).anyTimes();
+ replay(deviceService);
+ assertTrue(rrp.noMoreEnabledPort(devId1, v10));
+ assertTrue(rrp.noMoreEnabledPort(devId1, v20));
+ assertTrue(rrp.noMoreEnabledPort(devId1, vInt));
+ }
+}
\ No newline at end of file
diff --git a/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/RoutingRulePopulator.java b/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/RoutingRulePopulator.java
index 94aed90..56b55ac 100644
--- a/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/RoutingRulePopulator.java
+++ b/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/RoutingRulePopulator.java
@@ -64,6 +64,7 @@
import java.util.Optional;
import java.util.Set;
import java.util.concurrent.atomic.AtomicLong;
+import java.util.stream.Collectors;
import static com.google.common.base.Preconditions.checkNotNull;
import static org.onlab.packet.Ethernet.TYPE_ARP;
@@ -837,16 +838,7 @@
// NOTE: Some switch hardware share the same filtering flow among different ports.
// We use this metadata to let the driver know that there is no more enabled port
// within the same VLAN on this device.
- boolean noMoreEnabledPort = srManager.interfaceService.getInterfaces().stream()
- .filter(intf -> intf.connectPoint().deviceId().equals(deviceId))
- .filter(intf -> intf.vlanTagged().contains(vlanId) ||
- intf.vlanUntagged().equals(vlanId) ||
- intf.vlanNative().equals(vlanId))
- .noneMatch(intf -> {
- Port port = srManager.deviceService.getPort(intf.connectPoint());
- return port != null && port.isEnabled();
- });
- if (noMoreEnabledPort) {
+ if (noMoreEnabledPort(deviceId, vlanId)) {
tBuilder.wipeDeferred();
}
@@ -1228,4 +1220,28 @@
}
}
}
+
+ /**
+ * Checks if there is other enabled port within the given VLAN on the given device.
+ *
+ * @param deviceId device ID
+ * @param vlanId VLAN ID
+ * @return true if there is no more port enabled within the given VLAN on the given device
+ */
+ boolean noMoreEnabledPort(DeviceId deviceId, VlanId vlanId) {
+ Set<ConnectPoint> enabledPorts = srManager.deviceService.getPorts(deviceId).stream()
+ .filter(Port::isEnabled)
+ .map(port -> new ConnectPoint(port.element().id(), port.number()))
+ .collect(Collectors.toSet());
+
+ return enabledPorts.stream().noneMatch(cp ->
+ // Given vlanId is included in the vlan-tagged configuration
+ srManager.getTaggedVlanId(cp).contains(vlanId) ||
+ // Given vlanId is INTERNAL_VLAN and the interface is not configured
+ (srManager.getTaggedVlanId(cp).isEmpty() && srManager.getInternalVlanId(cp) == null &&
+ vlanId.equals(INTERNAL_VLAN)) ||
+ // interface is configured and either vlan-untagged or vlan-native matches given vlanId
+ (srManager.getInternalVlanId(cp) != null && srManager.getInternalVlanId(cp).equals(vlanId))
+ );
+ }
}