[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)
This commit is contained in:
pierventre 2021-09-23 19:03:14 +02:00 committed by Pier Luigi Ventre
parent 7e41399499
commit 581c8407e6
3 changed files with 43 additions and 12 deletions

View File

@ -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;
}
}

View File

@ -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" />

View File

@ -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));
}