Adding a higher priority flow to trap dhcp messages from server at dhcpServerConnectPoint (agg-switch uplink)
- The flows added previously were being ignored due to the vlan-crossconnect functionality
in the agg-switch. The priority of those flows were too low.
- Also added a component config to ensure that ONOS core HostLocationProvider learns
IP addresses of hosts from dhcp ACK messages.
Change-Id: Ic4377e5ff5642bd05117d9ad286f50f94cc51ec5
diff --git a/src/main/java/org/opencord/dhcpl2relay/DhcpL2Relay.java b/src/main/java/org/opencord/dhcpl2relay/DhcpL2Relay.java
index a577ae8..9f68554 100755
--- a/src/main/java/org/opencord/dhcpl2relay/DhcpL2Relay.java
+++ b/src/main/java/org/opencord/dhcpl2relay/DhcpL2Relay.java
@@ -40,6 +40,7 @@
import org.apache.felix.scr.annotations.Service;
import org.onlab.packet.DHCP;
import org.onlab.packet.DHCPPacketType;
+import org.onlab.packet.EthType.EtherType;
import org.onlab.packet.Ethernet;
import org.onlab.packet.IPv4;
import org.onlab.packet.IpAddress;
@@ -74,6 +75,9 @@
import org.onosproject.net.flow.DefaultTrafficTreatment;
import org.onosproject.net.flow.TrafficSelector;
import org.onosproject.net.flow.TrafficTreatment;
+import org.onosproject.net.flowobjective.DefaultForwardingObjective;
+import org.onosproject.net.flowobjective.FlowObjectiveService;
+import org.onosproject.net.flowobjective.ForwardingObjective;
import org.onosproject.net.host.HostService;
import org.onosproject.net.packet.DefaultOutboundPacket;
import org.onosproject.net.packet.OutboundPacket;
@@ -103,6 +107,8 @@
implements DhcpL2RelayService {
public static final String DHCP_L2RELAY_APP = "org.opencord.dhcpl2relay";
+ private static final String HOST_LOC_PROVIDER =
+ "org.onosproject.provider.host.impl.HostLocationProvider";
private final Logger log = LoggerFactory.getLogger(getClass());
private final InternalConfigListener cfgListener =
new InternalConfigListener();
@@ -142,6 +148,9 @@
@Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
protected MastershipService mastershipService;
+ @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
+ protected FlowObjectiveService flowObjectiveService;
+
@Property(name = "option82", boolValue = true,
label = "Add option 82 to relayed packets")
protected boolean option82 = true;
@@ -171,6 +180,9 @@
protected void activate(ComponentContext context) {
//start the dhcp relay agent
appId = coreService.registerApplication(DHCP_L2RELAY_APP);
+ // ensure that host-learning via dhcp includes IP addresses
+ componentConfigService.preSetProperty(HOST_LOC_PROVIDER,
+ "useDhcp", Boolean.TRUE.toString());
componentConfigService.registerProperties(getClass());
eventDispatcher.addSink(DhcpL2RelayEvent.class, listenerRegistry);
@@ -184,7 +196,6 @@
//add the packet services.
packetService.addProcessor(dhcpRelayPacketProcessor,
PacketProcessor.director(0));
- requestDhcpPackets();
if (context != null) {
modified(context);
}
@@ -197,7 +208,7 @@
cfgService.removeListener(cfgListener);
factories.forEach(cfgService::unregisterConfigFactory);
packetService.removeProcessor(dhcpRelayPacketProcessor);
- cancelDhcpPackets();
+ cancelDhcpPktsFromServer();
componentConfigService.unregisterProperties(getClass(), false);
deviceService.removeListener(deviceListener);
@@ -273,13 +284,95 @@
dhcpConnectPoints = Sets.newConcurrentHashSet(cfg.getDhcpServerConnectPoint());
modifyClientPktsSrcDstMac = cfg.getModifySrcDstMacAddresses();
+ boolean prevUseOltUplink = useOltUplink;
useOltUplink = cfg.getUseOltUplinkForServerPktInOut();
- if (!useOltUplink) {
+ if (useOltUplink) {
+ for (ConnectPoint cp : getUplinkPortsOfOlts()) {
+ log.debug("requestDhcpPackets: ConnectPoint: {}", cp);
+ requestDhcpPacketsFromConnectPoint(cp);
+ }
+ // check if previous config was different and so trap flows may
+ // need to be removed from other places
+ if (!prevUseOltUplink) {
+ if (dhcpServerConnectPoint.get() == null) {
+ log.warn("No dhcpServer connectPoint found .. cannot remove"
+ + " previously installed trap flows");
+ return;
+ }
+ // XXX change to use packet service when priority can be set
+ DefaultForwardingObjective.Builder bldr =
+ trapDhcpPktsFromServer(dhcpServerConnectPoint.get().deviceId(),
+ dhcpServerConnectPoint.get().port());
+ flowObjectiveService.forward(dhcpServerConnectPoint.get().deviceId(),
+ bldr.remove());
+ }
+
+ } else {
selectServerConnectPoint();
+ log.info("dhcp server connect point: " + dhcpServerConnectPoint);
+ // create flow to trap packets coming from the server to this connectpoint
+ // which is typically not on the OLT (otherwise use the OLT's uplink)
+ if (dhcpServerConnectPoint.get() == null) {
+ log.warn("No dhcpServer connectPoint found, cannot install dhcp"
+ + " trap flows");
+ return;
+ }
+ // XXX change to use packet service when priority can be set
+ DefaultForwardingObjective.Builder bldr =
+ trapDhcpPktsFromServer(dhcpServerConnectPoint.get().deviceId(),
+ dhcpServerConnectPoint.get().port());
+ flowObjectiveService.forward(dhcpServerConnectPoint.get().deviceId(),
+ bldr.add());
+ }
+ }
+
+ private void cancelDhcpPktsFromServer() {
+ if (useOltUplink) {
+ for (ConnectPoint cp : getUplinkPortsOfOlts()) {
+ log.debug("cancelDhcpPackets: ConnectPoint: {}", cp);
+ cancelDhcpPacketsFromConnectPoint(cp);
+ }
+ } else {
+ // delete flow to trap packets coming from the server to this connectpoint
+ // which is typically not on the OLT (otherwise use the OLT's uplink)
+ if (dhcpServerConnectPoint.get() == null) {
+ log.warn("No dhcpServer connectPoint found.. cannot remove dhcp"
+ + " trap flows");
+ return;
+ }
+ // XXX change to use packet service when priority can be set
+ DefaultForwardingObjective.Builder bldr =
+ trapDhcpPktsFromServer(dhcpServerConnectPoint.get().deviceId(),
+ dhcpServerConnectPoint.get().port());
+ flowObjectiveService.forward(dhcpServerConnectPoint.get().deviceId(),
+ bldr.remove());
}
- log.info("dhcp server connect point: " + dhcpServerConnectPoint);
+ }
+
+ private DefaultForwardingObjective.Builder trapDhcpPktsFromServer(DeviceId devId,
+ PortNumber port) {
+ TrafficTreatment treatment = DefaultTrafficTreatment.builder()
+ .punt()
+ .wipeDeferred()
+ .build();
+ TrafficSelector downstream = DefaultTrafficSelector.builder()
+ .matchInPort(port)
+ .matchEthType(EtherType.IPV4.ethType().toShort())
+ .matchIPProtocol(IPv4.PROTOCOL_UDP)
+ .matchUdpSrc(TpPort.tpPort(67))
+ .matchUdpDst(TpPort.tpPort(68))
+ .build();
+
+ DefaultForwardingObjective.Builder bldr = DefaultForwardingObjective.builder()
+ .withPriority(61000) // XXX packet service does not allow setting this
+ .withSelector(downstream)
+ .fromApp(appId)
+ .withFlag(ForwardingObjective.Flag.VERSATILE)
+ .withTreatment(treatment)
+ .makePermanent();
+ return bldr;
}
/**
@@ -382,58 +475,6 @@
PacketPriority.CONTROL, appId, Optional.of(cp.deviceId()));
}
- /**
- * Request DHCP packet in via PacketService.
- */
- private void requestDhcpPackets() {
- if (!useOltUplink) {
- TrafficSelector.Builder selectorServer = DefaultTrafficSelector.builder()
- .matchEthType(Ethernet.TYPE_IPV4)
- .matchIPProtocol(IPv4.PROTOCOL_UDP)
- .matchUdpSrc(TpPort.tpPort(UDP.DHCP_SERVER_PORT));
- packetService.requestPackets(selectorServer.build(),
- PacketPriority.CONTROL, appId);
-
- TrafficSelector.Builder selectorClient = DefaultTrafficSelector.builder()
- .matchEthType(Ethernet.TYPE_IPV4)
- .matchIPProtocol(IPv4.PROTOCOL_UDP)
- .matchUdpSrc(TpPort.tpPort(UDP.DHCP_CLIENT_PORT));
- packetService.requestPackets(selectorClient.build(),
- PacketPriority.CONTROL, appId);
- } else {
- for (ConnectPoint cp: getUplinkPortsOfOlts()) {
- log.debug("requestDhcpPackets: ConnectPoint: {}", cp);
- requestDhcpPacketsFromConnectPoint(cp);
- }
- }
-
- }
-
- /**
- * Cancel requested DHCP packets in via packet service.
- */
- private void cancelDhcpPackets() {
- if (!useOltUplink) {
- TrafficSelector.Builder selectorServer = DefaultTrafficSelector.builder()
- .matchEthType(Ethernet.TYPE_IPV4)
- .matchIPProtocol(IPv4.PROTOCOL_UDP)
- .matchUdpSrc(TpPort.tpPort(UDP.DHCP_SERVER_PORT));
- packetService.cancelPackets(selectorServer.build(),
- PacketPriority.CONTROL, appId);
-
- TrafficSelector.Builder selectorClient = DefaultTrafficSelector.builder()
- .matchEthType(Ethernet.TYPE_IPV4)
- .matchIPProtocol(IPv4.PROTOCOL_UDP)
- .matchUdpSrc(TpPort.tpPort(UDP.DHCP_CLIENT_PORT));
- packetService.cancelPackets(selectorClient.build(),
- PacketPriority.CONTROL, appId);
- } else {
- for (ConnectPoint cp: getUplinkPortsOfOlts()) {
- cancelDhcpPacketsFromConnectPoint(cp);
- }
- }
- }
-
public static Map<String, DhcpAllocationInfo> allocationMap() {
return allocationMap;
}
@@ -923,7 +964,7 @@
} else {
switch (event.type()) {
case PORT_ADDED:
- if (isUplinkPortOfOlt(event.subject().id(), event.port())) {
+ if (useOltUplink && isUplinkPortOfOlt(event.subject().id(), event.port())) {
requestDhcpPacketsFromConnectPoint(new ConnectPoint(event.subject().id(),
event.port().number()));
}
diff --git a/src/test/java/org/opencord/dhcpl2relay/DhcpL2RelayTest.java b/src/test/java/org/opencord/dhcpl2relay/DhcpL2RelayTest.java
index f3aea3e..b282728 100755
--- a/src/test/java/org/opencord/dhcpl2relay/DhcpL2RelayTest.java
+++ b/src/test/java/org/opencord/dhcpl2relay/DhcpL2RelayTest.java
@@ -15,8 +15,12 @@
*/
package org.opencord.dhcpl2relay;
-import com.google.common.collect.ImmutableSet;
-import com.google.common.collect.Lists;
+import static org.junit.Assert.assertEquals;
+
+import java.nio.ByteBuffer;
+import java.util.List;
+import java.util.Set;
+
import org.easymock.EasyMock;
import org.junit.After;
import org.junit.Before;
@@ -35,7 +39,6 @@
import org.onlab.packet.dhcp.DhcpOption;
import org.onosproject.cfg.ComponentConfigService;
import org.onosproject.common.event.impl.TestEventDispatcher;
-import org.onosproject.core.CoreServiceAdapter;
import org.onosproject.mastership.MastershipServiceAdapter;
import org.onosproject.net.AnnotationKeys;
import org.onosproject.net.Annotations;
@@ -54,6 +57,7 @@
import org.onosproject.net.config.Config;
import org.onosproject.net.config.NetworkConfigRegistryAdapter;
import org.onosproject.net.device.DeviceServiceAdapter;
+import org.onosproject.net.flowobjective.FlowObjectiveServiceAdapter;
import org.onosproject.net.host.HostServiceAdapter;
import org.onosproject.net.provider.ProviderId;
import org.opencord.dhcpl2relay.packet.DhcpOption82;
@@ -62,11 +66,8 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-import java.nio.ByteBuffer;
-import java.util.List;
-import java.util.Set;
-
-import static org.junit.Assert.assertEquals;
+import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Lists;
public class DhcpL2RelayTest extends DhcpL2RelayTestBase {
@@ -101,7 +102,8 @@
public void setUp() {
dhcpL2Relay = new DhcpL2Relay();
dhcpL2Relay.cfgService = new TestNetworkConfigRegistry();
- dhcpL2Relay.coreService = new CoreServiceAdapter();
+ dhcpL2Relay.coreService = new MockCoreServiceAdapter();
+ dhcpL2Relay.flowObjectiveService = new FlowObjectiveServiceAdapter();
dhcpL2Relay.packetService = new MockPacketService();
dhcpL2Relay.componentConfigService = mockConfigService;
dhcpL2Relay.deviceService = new MockDeviceService();
@@ -269,18 +271,23 @@
public boolean isEnabled() {
return true;
}
+ @Override
public long portSpeed() {
return 1000;
}
+ @Override
public Element element() {
return null;
}
+ @Override
public PortNumber number() {
return null;
}
+ @Override
public Annotations annotations() {
return new MockAnnotations();
}
+ @Override
public Type type() {
return Port.Type.FIBER;
}
@@ -291,6 +298,7 @@
public String value(String val) {
return "PON 1/1";
}
+ @Override
public Set<String> keys() {
return null;
}
@@ -315,7 +323,9 @@
@Override
public void invalidateAll() {}
+ @Override
public void invalidateId(String id) {}
+ @Override
public SubscriberAndDeviceInformation getfromCache(String id) {
return null;
}
diff --git a/src/test/java/org/opencord/dhcpl2relay/DhcpL2RelayTestBase.java b/src/test/java/org/opencord/dhcpl2relay/DhcpL2RelayTestBase.java
index 7a1dd1a..fb519cf 100755
--- a/src/test/java/org/opencord/dhcpl2relay/DhcpL2RelayTestBase.java
+++ b/src/test/java/org/opencord/dhcpl2relay/DhcpL2RelayTestBase.java
@@ -16,6 +16,14 @@
package org.opencord.dhcpl2relay;
+import static org.junit.Assert.fail;
+
+import java.nio.ByteBuffer;
+import java.nio.charset.StandardCharsets;
+import java.util.ArrayList;
+import java.util.LinkedList;
+import java.util.List;
+
import org.onlab.packet.BasePacket;
import org.onlab.packet.DHCP;
import org.onlab.packet.DHCPPacketType;
@@ -25,6 +33,9 @@
import org.onlab.packet.MacAddress;
import org.onlab.packet.UDP;
import org.onlab.packet.dhcp.DhcpOption;
+import org.onosproject.core.ApplicationId;
+import org.onosproject.core.CoreServiceAdapter;
+import org.onosproject.core.DefaultApplicationId;
import org.onosproject.net.ConnectPoint;
import org.onosproject.net.packet.DefaultInboundPacket;
import org.onosproject.net.packet.DefaultPacketContext;
@@ -36,14 +47,6 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-import java.nio.ByteBuffer;
-import java.nio.charset.StandardCharsets;
-import java.util.ArrayList;
-import java.util.LinkedList;
-import java.util.List;
-
-import static org.junit.Assert.fail;
-
/**
* Common methods for AAA app testing.
@@ -73,6 +76,17 @@
}
/**
+ * Mock core service adaptor that provides an appId.
+ */
+ class MockCoreServiceAdapter extends CoreServiceAdapter {
+
+ @Override
+ public ApplicationId registerApplication(String name) {
+ return new DefaultApplicationId(10, name);
+ }
+ }
+
+ /**
* Keeps a reference to the PacketProcessor and saves the OutboundPackets.
*/
class MockPacketService extends PacketServiceAdapter {