From 581c8407e613a27aaa573a600828b30a37066fb8 Mon Sep 17 00:00:00 2001 From: pierventre Date: Thu, 23 Sep 2021 19:03:14 +0200 Subject: [PATCH] [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) --- .../dhcprelay/Dhcp4HandlerImpl.java | 28 +++++++++++++++++-- .../src/main/resources/onos/suppressions.xml | 1 + .../java/org/onlab/packet/dhcp/CircuitId.java | 26 +++++++++++------ 3 files changed, 43 insertions(+), 12 deletions(-) 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 7325010137..6cd70de8c0 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.DeviceId; 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 @@ public class Dhcp4HandlerImpl implements DhcpHandler, HostProvider { 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 @@ public class Dhcp4HandlerImpl implements DhcpHandler, HostProvider { 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 @@ public class Dhcp4HandlerImpl implements DhcpHandler, HostProvider { } 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 3cd63b1de5..c972216a01 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 @@ + 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 1b53a5b9dc..22342465ab 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 @@ import static com.google.common.base.Preconditions.checkArgument; 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 @@ public class CircuitId { // 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)); }