[CORD-1779] Fix directlyConnected function
Check circuit ID format to determind the option added by ONOS or not
instead of using giaddr.
Change-Id: Ibb20059d37fe036a21c71d38ac771b0613441f2d
diff --git a/apps/dhcprelay/src/main/java/org/onosproject/dhcprelay/Dhcp4HandlerImpl.java b/apps/dhcprelay/src/main/java/org/onosproject/dhcprelay/Dhcp4HandlerImpl.java
index deeaa36..4d2bebb 100644
--- a/apps/dhcprelay/src/main/java/org/onosproject/dhcprelay/Dhcp4HandlerImpl.java
+++ b/apps/dhcprelay/src/main/java/org/onosproject/dhcprelay/Dhcp4HandlerImpl.java
@@ -675,23 +675,24 @@
* @return true if the host is directly connected to the network; false otherwise
*/
private boolean directlyConnected(DHCP dhcpPayload) {
- DhcpOption relayAgentOption = dhcpPayload.getOption(OptionCode_CircuitID);
+ DhcpRelayAgentOption relayAgentOption =
+ (DhcpRelayAgentOption) dhcpPayload.getOption(OptionCode_CircuitID);
// Doesn't contains relay option
if (relayAgentOption == null) {
return true;
}
- IpAddress gatewayIp = IpAddress.valueOf(dhcpPayload.getGatewayIPAddress());
- Set<Interface> gatewayInterfaces = interfaceService.getInterfacesByIp(gatewayIp);
+ // check circuit id, if circuit id is invalid, we say it is an indirect host
+ DhcpOption circuitIdOpt = relayAgentOption.getSubOption(CIRCUIT_ID.getValue());
- // Contains relay option, and added by ONOS
- if (!gatewayInterfaces.isEmpty()) {
+ try {
+ CircuitId.deserialize(circuitIdOpt.getData());
return true;
+ } catch (Exception e) {
+ // invalid circuit id
+ return false;
}
-
- // Relay option added by other relay agent
- return false;
}
diff --git a/apps/dhcprelay/src/test/java/org/onosproject/dhcprelay/DhcpRelayManagerTest.java b/apps/dhcprelay/src/test/java/org/onosproject/dhcprelay/DhcpRelayManagerTest.java
index 8a47cf8..71b3cef 100644
--- a/apps/dhcprelay/src/test/java/org/onosproject/dhcprelay/DhcpRelayManagerTest.java
+++ b/apps/dhcprelay/src/test/java/org/onosproject/dhcprelay/DhcpRelayManagerTest.java
@@ -20,6 +20,7 @@
import com.google.common.collect.Lists;
import com.google.common.collect.Maps;
import com.google.common.collect.Sets;
+import org.apache.commons.io.Charsets;
import org.easymock.EasyMock;
import org.junit.After;
import org.junit.Before;
@@ -159,6 +160,7 @@
CLIENT2_INTERFACE,
SERVER_INTERFACE
);
+ private static final String NON_ONOS_CID = "Non-ONOS circuit ID";
private DhcpRelayManager manager;
private MockPacketService packetService;
@@ -576,8 +578,8 @@
if (withNonOnosRelayInfo) {
DhcpRelayAgentOption relayOption = new DhcpRelayAgentOption();
DhcpOption circuitIdOption = new DhcpOption();
- CircuitId circuitId = new CircuitId("Custom cid", VlanId.NONE);
- byte[] cid = circuitId.serialize();
+ String circuitId = NON_ONOS_CID;
+ byte[] cid = circuitId.getBytes(Charsets.US_ASCII);
circuitIdOption.setCode(DhcpRelayAgentOption.RelayAgentInfoOptions.CIRCUIT_ID.getValue());
circuitIdOption.setLength((byte) cid.length);
circuitIdOption.setData(cid);
diff --git a/utils/misc/src/main/java/org/onlab/packet/dhcp/CircuitId.java b/utils/misc/src/main/java/org/onlab/packet/dhcp/CircuitId.java
index 3b819e1..1b53a5b 100644
--- a/utils/misc/src/main/java/org/onlab/packet/dhcp/CircuitId.java
+++ b/utils/misc/src/main/java/org/onlab/packet/dhcp/CircuitId.java
@@ -17,6 +17,7 @@
package org.onlab.packet.dhcp;
import com.google.common.collect.Lists;
+import com.google.common.primitives.UnsignedLongs;
import org.onlab.packet.VlanId;
import java.nio.charset.StandardCharsets;
@@ -31,6 +32,7 @@
public class CircuitId {
private static final String SEPARATOR = ":";
private static final String CIRCUIT_ID_FORMAT = "%s" + SEPARATOR + "%s";
+ private static final String DEVICE_PORT_SEPARATOR = "/";
private String connectPoint;
private VlanId vlanId;
@@ -65,11 +67,22 @@
*/
public static CircuitId deserialize(byte[] circuitId) {
String cIdString = new String(circuitId, StandardCharsets.US_ASCII);
- List<String> split = Lists.newArrayList(cIdString.split(SEPARATOR));
- checkArgument(split.size() > 1, "Illegal circuit id.");
+ List<String> splittedCircuitId = Lists.newArrayList(cIdString.split(SEPARATOR));
+ checkArgument(splittedCircuitId.size() > 1, "Illegal circuit id.");
// remove last element (vlan id)
- String vlanId = split.remove(split.size() - 1);
- String connectPoint = String.join(SEPARATOR, split);
+ String vlanId = splittedCircuitId.remove(splittedCircuitId.size() - 1);
+
+ // Reconstruct device Id
+ String connectPoint = String.join(SEPARATOR, splittedCircuitId);
+
+ String[] splittedConnectPoint = connectPoint.split(DEVICE_PORT_SEPARATOR);
+ // Check connect point is valid or not
+ checkArgument(splittedConnectPoint.length == 2,
+ "Connect point must be in \"deviceUri/portNumber\" format");
+
+ // Check the port number is a number or not
+ UnsignedLongs.decode(splittedConnectPoint[1]);
+
return new CircuitId(connectPoint, VlanId.vlanId(vlanId));
}