VOL-3544 Enable the use of option82 for relaying packets to client without hostStore lookup.
Refactor the app's packet processor and some methods for more optimized packet handling,
for example avoid multiple lookups to the host store.
Change-Id: Id26cee017f14e2c838b9f42f20ebb3ec2ddc5efa
diff --git a/app/src/test/java/org/opencord/dhcpl2relay/impl/DhcpL2RelayTest.java b/app/src/test/java/org/opencord/dhcpl2relay/impl/DhcpL2RelayTest.java
index c19b2eb..3e19917 100755
--- a/app/src/test/java/org/opencord/dhcpl2relay/impl/DhcpL2RelayTest.java
+++ b/app/src/test/java/org/opencord/dhcpl2relay/impl/DhcpL2RelayTest.java
@@ -16,19 +16,21 @@
package org.opencord.dhcpl2relay.impl;
import static org.easymock.EasyMock.createMock;
-import static org.junit.Assert.*;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.fail;
import static org.slf4j.LoggerFactory.getLogger;
import java.nio.ByteBuffer;
import java.util.List;
import java.util.Map;
+import java.util.stream.Collectors;
-import com.google.common.util.concurrent.MoreExecutors;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;
import org.onlab.junit.TestUtils;
import org.onlab.packet.DHCP;
+import org.onlab.packet.DeserializationException;
import org.onlab.packet.Ethernet;
import org.onlab.packet.IPv4;
import org.onlab.packet.MacAddress;
@@ -45,10 +47,11 @@
import org.opencord.dhcpl2relay.DhcpAllocationInfo;
import org.opencord.dhcpl2relay.DhcpL2RelayEvent;
import org.opencord.dhcpl2relay.DhcpL2RelayStoreDelegate;
-import org.opencord.dhcpl2relay.impl.packet.DhcpOption82;
+import org.opencord.dhcpl2relay.impl.packet.DhcpOption82Data;
import org.slf4j.Logger;
import com.google.common.collect.Lists;
+import com.google.common.util.concurrent.MoreExecutors;
public class DhcpL2RelayTest extends DhcpL2RelayTestBase {
@@ -101,9 +104,9 @@
dhcpL2Relay.deactivate();
}
- private void checkAllocation(DHCP.MsgType messageType) {
+ private void checkAllocation(DHCP.MsgType messageType, String circuitId) {
ConnectPoint clientCp = ConnectPoint.deviceConnectPoint(OLT_DEV_ID + "/"
- + String.valueOf(CLIENT_PORT));
+ + String.valueOf(CLIENT_PORT));
allocs = dhcpL2Relay.getAllocationInfo();
assertEquals(1, allocs.size());
allocs.forEach((k, v) -> {
@@ -111,6 +114,7 @@
assertEquals(v.type(), messageType);
assertEquals(v.macAddress(), CLIENT_MAC);
assertEquals(v.location(), clientCp);
+ assertEquals(v.circuitId(), circuitId);
});
}
@@ -175,7 +179,8 @@
}
/**
- * Tests the DHCP relay app by sending DHCP discovery Packet.
+ * Tests the DHCP relay app by sending DHCP discovery Packet. The circuitId
+ * and remote-Id for this client is operator defined in MockSadis.
*
* @throws Exception when an unhandled error occurs
*/
@@ -185,12 +190,79 @@
dhcpL2Relay.clearAllocations();
Ethernet discoverPacket = constructDhcpDiscoverPacket(CLIENT_MAC);
ConnectPoint clientCp = ConnectPoint.deviceConnectPoint(OLT_DEV_ID + "/"
- + String.valueOf(CLIENT_PORT));
- sendPacket(discoverPacket, clientCp);
+ + String.valueOf(CLIENT_PORT));
+ // send a copy of the packet as the app code modifies the sent packet
+ sendPacket(discoverPacket.duplicate(), clientCp);
Ethernet discoverRelayed = (Ethernet) getPacket();
compareClientPackets(discoverPacket, discoverRelayed);
- checkAllocation(DHCP.MsgType.DHCPDISCOVER);
+ checkAllocation(DHCP.MsgType.DHCPDISCOVER, CLIENT_CIRCUIT_ID);
+ }
+
+ /**
+ * Tests the addition of app-defined circuit id, when this client's
+ * MockSadis config for circiutId is empty. The remoteId is configured.
+ *
+ * @throws Exception when an unhandled error occurs
+ */
+ @Test
+ public void testDhcpDiscoverEmptyCircuitId() throws Exception {
+ dhcpL2Relay.clearAllocations();
+ MacAddress mac32 = MacAddress.valueOf("b4:96:91:0c:4f:e4");
+ VlanId vlan32a = VlanId.vlanId((short) 801); // defined in mockSadis
+ VlanId qinq32a = VlanId.vlanId((short) 111);
+ Ethernet discover32a = constructDhcpDiscoverPacket(mac32, vlan32a,
+ (short) 0);
+ ConnectPoint client32 = ConnectPoint
+ .deviceConnectPoint("of:0000b86a974385f7/32");
+ sendPacket(discover32a.duplicate(), client32);
+ Ethernet discoverRelayed = (Ethernet) getPacket();
+ // empty circuitId in sadis for client32 should result in app defined
+ // circuitId
+ String expectedCircuitId = client32 + ":vlan" + vlan32a + ":pcp-1";
+ compareClientPackets(discover32a, discoverRelayed,
+ qinq32a, vlan32a, CLIENT_C_PBIT,
+ expectedCircuitId,
+ CLIENT_REMOTE_ID);
+ allocs = dhcpL2Relay.getAllocationInfo();
+ allocs.forEach((k, v) -> {
+ log.info("Allocation {} : {}", k, v);
+ assertEquals(v.circuitId(), expectedCircuitId);
+ });
+ }
+
+ /**
+ * Tests the addition of app-defined circuit id, when this client's
+ * MockSadis config for circuitId and remoteId are null. In addition, it
+ * tests that the configured downstream-pcp is included in the circuitId.
+ *
+ * @throws Exception when an unhandled error occurs
+ */
+ @Test
+ public void testDhcpDiscoverNullIds() throws Exception {
+ dhcpL2Relay.clearAllocations();
+ MacAddress mac4112 = MacAddress.valueOf("b4:96:91:0c:4f:c9");
+ VlanId vlan4112 = VlanId.vlanId((short) 101);
+ VlanId qinq4112 = VlanId.vlanId((short) 222);
+ Ethernet discover4112 = constructDhcpDiscoverPacket(mac4112, vlan4112,
+ (short) 0);
+ ConnectPoint client4112 = ConnectPoint
+ .deviceConnectPoint("of:0000b86a974385f7/4112");
+ sendPacket(discover4112.duplicate(), client4112);
+ Ethernet discoverRelayed = (Ethernet) getPacket();
+ // null circuitId in sadis for client32 should result in app defined
+ // circuitId. remoteId should not be there. Correct downstream pbit
+ // should be used
+ String expectedCircuitId = client4112 + ":vlan" + vlan4112 + ":pcp5";
+ compareClientPackets(discover4112, discoverRelayed,
+ qinq4112, vlan4112, CLIENT_C_PBIT,
+ expectedCircuitId,
+ null);
+ allocs = dhcpL2Relay.getAllocationInfo();
+ allocs.forEach((k, v) -> {
+ log.info("Allocation {} : {}", k, v);
+ assertEquals(v.circuitId(), expectedCircuitId);
+ });
}
/**
@@ -203,67 +275,96 @@
// Sending DHCP Request packet
Ethernet requestPacket = constructDhcpRequestPacket(CLIENT_MAC);
ConnectPoint clientCp = ConnectPoint.deviceConnectPoint(OLT_DEV_ID + "/"
- + String.valueOf(CLIENT_PORT));
- sendPacket(requestPacket, clientCp);
+ + String.valueOf(CLIENT_PORT));
+ sendPacket(requestPacket.duplicate(), clientCp);
Ethernet requestRelayed = (Ethernet) getPacket();
compareClientPackets(requestPacket, requestRelayed);
- checkAllocation(DHCP.MsgType.DHCPREQUEST);
+ checkAllocation(DHCP.MsgType.DHCPREQUEST, CLIENT_CIRCUIT_ID);
}
/**
- * Tests the DHCP relay app by sending DHCP Offer Packet.
+ * Tests the DHCP relay app by sending DHCP Offer Packet with app-defined
+ * circuit id. App should use the circuit id for forwarding.
*
* @throws Exception when an unhandled error occurs
*/
@Test
public void testDhcpOffer() throws InterruptedException {
// Sending DHCP Offer packet
- Ethernet offerPacket = constructDhcpOfferPacket(SERVER_MAC,
- CLIENT_MAC, DESTINATION_ADDRESS_IP, DHCP_CLIENT_IP_ADDRESS);
+ Ethernet offerPacket = constructDhcpOfferPacket(SERVER_MAC, CLIENT_MAC,
+ DESTINATION_ADDRESS_IP,
+ DHCP_CLIENT_IP_ADDRESS);
sendPacket(offerPacket, ConnectPoint.deviceConnectPoint(OLT_DEV_ID + "/"
- + String.valueOf(UPLINK_PORT)));
+ + String.valueOf(UPLINK_PORT)));
Ethernet offerRelayed = (Ethernet) getPacket();
compareServerPackets(offerPacket, offerRelayed);
- checkAllocation(DHCP.MsgType.DHCPOFFER);
+ String expectedCircuitId = OLT_DEV_ID + "/" + CLIENT_PORT + ":vlan"
+ + CLIENT_C_TAG + ":pcp" + CLIENT_C_PBIT;
+ checkAllocation(DHCP.MsgType.DHCPOFFER, expectedCircuitId);
}
/**
- * Tests the DHCP relay app by sending DHCP Ack Packet.
+ * Tests the DHCP relay app by sending DHCP Ack Packet with operator defined
+ * circuit id. App should ignore circuit Id and do a host lookup.
*
* @throws Exception when an unhandled error occurs
*/
@Test
public void testDhcpAck() throws InterruptedException {
- Ethernet ackPacket = constructDhcpAckPacket(SERVER_MAC,
- CLIENT_MAC, DESTINATION_ADDRESS_IP, DHCP_CLIENT_IP_ADDRESS);
+ Ethernet ackPacket = constructDhcpAckPacket(SERVER_MAC, CLIENT_MAC,
+ DESTINATION_ADDRESS_IP,
+ DHCP_CLIENT_IP_ADDRESS);
sendPacket(ackPacket, ConnectPoint.deviceConnectPoint(OLT_DEV_ID + "/"
- + String.valueOf(UPLINK_PORT)));
+ + String.valueOf(UPLINK_PORT)));
Ethernet ackRelayed = (Ethernet) getPacket();
compareServerPackets(ackPacket, ackRelayed);
- checkAllocation(DHCP.MsgType.DHCPACK);
+ checkAllocation(DHCP.MsgType.DHCPACK, CLIENT_CIRCUIT_ID);
}
/**
* Tests the DHCP relay app by sending DHCP Nak Packet.
+ * Tests app-defined option82, but uses incorrect connectPoint - packet
+ * should still be forwarded to this connectPoint (ie without host lookup).
+ * Also pbit in circuitId is -1, which means original pbit should be retained
*
* @throws Exception when an unhandled error occurs
*/
@Test
public void testDhcpNak() throws InterruptedException {
+ VlanId fakeVlan = VlanId.vlanId((short) 50);
+ short fakePcp = (short) 4; // should be retained
+ VlanId expectedVlan = VlanId.vlanId((short) 111);
+ // relayed packet should have vlan 111 and retain pcp4 and be sent out
+ // of port32
+ ConnectPoint fakeCp = ConnectPoint.fromString("of:0000b86a974385f7/32");
+ String fakeCircuitId = fakeCp + ":vlan"
+ + expectedVlan + ":pcp-1";
+ Ethernet nakPacket = constructDhcpNakPacket(SERVER_MAC, CLIENT_MAC,
+ DESTINATION_ADDRESS_IP,
+ DHCP_CLIENT_IP_ADDRESS,
+ fakeVlan,
+ fakePcp);
- Ethernet nakPacket = constructDhcpNakPacket(SERVER_MAC,
- CLIENT_MAC, DESTINATION_ADDRESS_IP, DHCP_CLIENT_IP_ADDRESS);
-
- sendPacket(nakPacket, ConnectPoint.deviceConnectPoint(OLT_DEV_ID + "/" + 1));
+ sendPacket(nakPacket, ConnectPoint.deviceConnectPoint(OLT_DEV_ID + "/"
+ + String.valueOf(UPLINK_PORT)));
Ethernet nakRelayed = (Ethernet) getPacket();
- compareServerPackets(nakPacket, nakRelayed);
- checkAllocation(DHCP.MsgType.DHCPNAK);
+ compareServerPackets(nakPacket, nakRelayed, expectedVlan, fakePcp);
+
+ allocs = dhcpL2Relay.getAllocationInfo();
+ assertEquals(1, allocs.size());
+ allocs.forEach((k, v) -> {
+ log.info("Allocation {} : {}", k, v);
+ assertEquals(v.type(), DHCP.MsgType.DHCPNAK);
+ assertEquals(v.macAddress(), CLIENT_MAC);
+ assertEquals(v.location(), fakeCp);
+ assertEquals(v.circuitId(), fakeCircuitId);
+ });
}
/**
@@ -276,11 +377,12 @@
Ethernet declinePacket = constructDhcpDeclinePacket(CLIENT_MAC);
- sendPacket(declinePacket, ConnectPoint.deviceConnectPoint(OLT_DEV_ID + "/" + 1));
+ sendPacket(declinePacket.duplicate(),
+ ConnectPoint.deviceConnectPoint(OLT_DEV_ID + "/" + 1));
Ethernet declineRelayed = (Ethernet) getPacket();
compareClientPackets(declinePacket, declineRelayed);
- checkAllocation(DHCP.MsgType.DHCPDECLINE);
+ checkAllocation(DHCP.MsgType.DHCPDECLINE, CLIENT_CIRCUIT_ID);
}
/**
@@ -352,18 +454,31 @@
}
public void compareClientPackets(Ethernet sent, Ethernet relayed) {
- sent.setSourceMACAddress(OLT_MAC_ADDRESS);
- sent.setQinQVID(CLIENT_S_TAG.toShort());
- sent.setVlanID(CLIENT_C_TAG.toShort());
- sent.setPriorityCode((byte) CLIENT_C_PBIT);
+ compareClientPackets(sent, relayed, CLIENT_S_TAG, CLIENT_C_TAG,
+ CLIENT_C_PBIT, CLIENT_CIRCUIT_ID,
+ CLIENT_REMOTE_ID);
+ }
+
+ public void compareClientPackets(Ethernet sent, Ethernet relayed,
+ VlanId expectedQinQ,
+ VlanId expectedVlan, short expectedPcp,
+ String expectedCircuitId,
+ String expectedRemoteId) {
+ // convert the sent packet to the expected relayed packet
+ sent.setSourceMACAddress(OLT_MAC_ADDRESS); // due to netconfig test in setup
+ sent.setQinQVID(expectedQinQ.toShort());
+ sent.setQinQTPID((short) 0x8100);
+ sent.setVlanID(expectedVlan.toShort());
+ sent.setPriorityCode((byte) expectedPcp);
IPv4 ipv4Packet = (IPv4) sent.getPayload();
UDP udpPacket = (UDP) ipv4Packet.getPayload();
DHCP dhcpPacket = (DHCP) udpPacket.getPayload();
-
List<DhcpOption> options = Lists.newArrayList(dhcpPacket.getOptions());
- DhcpOption82 option82 = new DhcpOption82();
- option82.setAgentCircuitId(CLIENT_CIRCUIT_ID);
+
+ DhcpOption82Data option82 = new DhcpOption82Data();
+ option82.setAgentCircuitId(expectedCircuitId);
+ option82.setAgentRemoteId(expectedRemoteId);
DhcpOption option = new DhcpOption()
.setCode(DHCP.DHCPOptionCode.OptionCode_CircuitID.getValue())
@@ -372,17 +487,54 @@
options.add(options.size() - 1, option);
dhcpPacket.setOptions(options);
- assertEquals(sent, relayed);
+ byte[] sb = sent.serialize();
+ Ethernet expectedPacket = null;
+ try {
+ expectedPacket = Ethernet.deserializer()
+ .deserialize(sb, 0, sb.length);
+ } catch (DeserializationException e) {
+ log.error("exeption: {}", e.getMessage());
+ fail();
+ }
+ verifyDhcpOptions(expectedPacket, relayed);
+ assertEquals(expectedPacket, relayed);
+ }
+
+ public void verifyDhcpOptions(Ethernet expected, Ethernet relayed) {
+ DHCP de = ((DHCP) ((UDP) ((IPv4) expected.getPayload()).getPayload())
+ .getPayload());
+ DHCP dr = ((DHCP) ((UDP) ((IPv4) relayed.getPayload()).getPayload())
+ .getPayload());
+ List<DhcpOption> del = de.getOptions();
+ List<DhcpOption> der = dr.getOptions();
+ assertEquals(del.size(), der.size());
+ for (int i = 0; i < del.size(); i++) {
+ assertEquals(del.get(i), der.get(i));
+ }
}
public void compareServerPackets(Ethernet sent, Ethernet relayed) {
+ compareServerPackets(sent, relayed, CLIENT_C_TAG, CLIENT_C_PBIT);
+ }
+ public void compareServerPackets(Ethernet sent, Ethernet relayed,
+ VlanId expectedVlan, short expectedPcp) {
try {
+ // modify sent packet to create expected packet
sent.setDestinationMACAddress(CLIENT_MAC);
sent.setQinQVID(NOT_PROVIDED);
sent.setQinQPriorityCode((byte) NOT_PROVIDED);
- sent.setVlanID(CLIENT_C_TAG.toShort());
+ sent.setVlanID(expectedVlan.toShort());
+ sent.setPriorityCode((byte) expectedPcp);
+ DHCP d = ((DHCP) ((UDP) ((IPv4) sent.getPayload()).getPayload())
+ .getPayload());
+ List<DhcpOption> newOptions = d.getOptions().stream()
+ .filter(option -> option
+ .getCode() != DHCP.DHCPOptionCode.OptionCode_CircuitID
+ .getValue())
+ .collect(Collectors.toList());
+ d.setOptions(newOptions);
final ByteBuffer byteBuffer = ByteBuffer.wrap(sent.serialize());
Ethernet expectedPacket = Ethernet.deserializer().deserialize(byteBuffer.array(),