[SDFAB-616] Inconsistent format of port number in DhcpRelay
CircuitId deserialize use custom parsing instead of leveraging
the ConnectPoint class. Unfortunately, this custom parsing
does not parse correctly the portname.
Additionally, fix port number format for hostlocation and dhcprecords
and exclude Dhcp4HandlerImpl from file length checks
Change-Id: I360f26f8dd7de492cb65ad7af05fb85c8e940c33
(cherry picked from commit 61bd673eec2282aff175daff141059870db78c7d)
diff --git a/apps/dhcprelay/app/src/main/java/org/onosproject/dhcprelay/Dhcp4HandlerImpl.java b/apps/dhcprelay/app/src/main/java/org/onosproject/dhcprelay/Dhcp4HandlerImpl.java
index 7325010..6cd70de 100644
--- a/apps/dhcprelay/app/src/main/java/org/onosproject/dhcprelay/Dhcp4HandlerImpl.java
+++ b/apps/dhcprelay/app/src/main/java/org/onosproject/dhcprelay/Dhcp4HandlerImpl.java
@@ -52,6 +52,7 @@
import org.onosproject.net.Host;
import org.onosproject.net.HostId;
import org.onosproject.net.HostLocation;
+import org.onosproject.net.Port;
import org.onosproject.net.behaviour.Pipeliner;
import org.onosproject.net.device.DeviceService;
import org.onosproject.net.flow.DefaultTrafficSelector;
@@ -1086,15 +1087,21 @@
log.warn("Failed to determine where to send {}", dhcpPayload.getPacketType());
return;
}
-
Interface outIface = outInterface.get();
+
ConnectPoint location = outIface.connectPoint();
+ if (!location.port().hasName()) {
+ location = translateSwitchPort(location);
+ }
+
VlanId vlanId = getVlanIdFromRelayAgentOption(dhcpPayload);
if (vlanId == null) {
vlanId = outIface.vlan();
}
+
MacAddress macAddress = MacAddress.valueOf(dhcpPayload.getClientHardwareAddress());
HostId hostId = HostId.hostId(macAddress, vlanId);
+
DhcpRecord record = dhcpRelayStore.getDhcpRecord(hostId).orElse(null);
if (record == null) {
record = new DhcpRecord(HostId.hostId(macAddress, vlanId));
@@ -1439,9 +1446,14 @@
log.warn("Can't find output interface for dhcp: {}", dhcpPayload);
return;
}
-
Interface outIface = outInterface.get();
- HostLocation hostLocation = new HostLocation(outIface.connectPoint(), System.currentTimeMillis());
+
+ ConnectPoint location = outIface.connectPoint();
+ if (!location.port().hasName()) {
+ location = translateSwitchPort(location);
+ }
+
+ HostLocation hostLocation = new HostLocation(location, System.currentTimeMillis());
MacAddress macAddress = MacAddress.valueOf(dhcpPayload.getClientHardwareAddress());
VlanId vlanId = getVlanIdFromRelayAgentOption(dhcpPayload);
if (vlanId == null) {
@@ -1993,4 +2005,14 @@
}
return foundServerInfo;
}
+
+ /* Connect point coming from interfaces do not have port name
+ we use the device service as translation service */
+ private ConnectPoint translateSwitchPort(ConnectPoint connectPoint) {
+ Port devicePort = deviceService.getPort(connectPoint);
+ if (devicePort != null) {
+ return new ConnectPoint(connectPoint.deviceId(), devicePort.number());
+ }
+ return connectPoint;
+ }
}
diff --git a/tools/build/conf/src/main/resources/onos/suppressions.xml b/tools/build/conf/src/main/resources/onos/suppressions.xml
index 3cd63b1..c972216 100644
--- a/tools/build/conf/src/main/resources/onos/suppressions.xml
+++ b/tools/build/conf/src/main/resources/onos/suppressions.xml
@@ -34,6 +34,7 @@
<suppress files="org.onosproject.segmentrouting.*" checks="AbbreviationAsWordInName" />
<suppress files="org.onosproject.segmentrouting.DefaultRoutingHandler.java" checks="FileLength" />
<suppress files="org.onosproject.segmentrouting.mcast.McastHandler" checks="FileLength" />
+ <suppress files="org.onosproject.dhcprelay.Dhcp4HandlerImpl" checks="FileLength" />
<!-- These files carry AT&T copyrights -->
<suppress files="org.onlab.packet.EAP" checks="RegexpHeader" />
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 1b53a5b..2234246 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,7 +17,6 @@
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;
@@ -32,7 +31,6 @@
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;
@@ -72,16 +70,26 @@
// remove last element (vlan id)
String vlanId = splittedCircuitId.remove(splittedCircuitId.size() - 1);
- // Reconstruct device Id
+ // Reconstruct the connect point string
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");
+ /*
+ * As device IDs may have a path component, we are expecting one
+ * of:
+ * - scheme:ip:port/cp
+ * - scheme:ip:port/path/cp
+ *
+ * The assumption is the last `/` will separate the device ID
+ * from the connection point number. Please note that we are
+ * not actually checking here the content of the strings
+ */
+ int idx = connectPoint.lastIndexOf("/");
+ checkArgument(idx != -1, "Connect point not specified, " +
+ "Connect point must be in \"deviceUri/portNumber\" format");
- // Check the port number is a number or not
- UnsignedLongs.decode(splittedConnectPoint[1]);
+ String cp = connectPoint.substring(idx + 1);
+ checkArgument(!cp.isEmpty(), "Connect point separator specified, " +
+ "but no port number included, connect point must be in \"deviceUri/portNumber\" format");
return new CircuitId(connectPoint, VlanId.vlanId(vlanId));
}