Refactoring ICMP handler in Segment Routing
- No longer handle ICMPv6 packets except those target routerIP and gatewayIP
- Remove source IP validation for ICMPv6 since IPv6 hosts often speak with their link local address
Change-Id: If92826c080a4643bad71b0d39fc89b7123dc11d5
diff --git a/src/main/java/org/onosproject/segmentrouting/IcmpHandler.java b/src/main/java/org/onosproject/segmentrouting/IcmpHandler.java
index a42a806..38293d0 100644
--- a/src/main/java/org/onosproject/segmentrouting/IcmpHandler.java
+++ b/src/main/java/org/onosproject/segmentrouting/IcmpHandler.java
@@ -23,7 +23,6 @@
import org.onlab.packet.Ip4Address;
import org.onlab.packet.Ip6Address;
import org.onlab.packet.IpAddress;
-import org.onlab.packet.IpPrefix;
import org.onlab.packet.MPLS;
import org.onlab.packet.MacAddress;
import org.onlab.packet.VlanId;
@@ -117,10 +116,8 @@
/**
* Process incoming ICMP packet.
- * If it is an ICMP request to router or known host, then sends an ICMP response.
- * If it is an ICMP packet to known host and forward the packet to the host.
- * If it is an ICMP packet to unknown host in a subnet, then sends an ARP request
- * to the subnet.
+ * If it is an ICMP request to router, then sends an ICMP response.
+ * Otherwise ignore the packet.
*
* @param eth inbound ICMP packet
* @param inPort the input port
@@ -142,24 +139,11 @@
(destinationAddress.equals(routerIp.getIp4Address()) ||
gatewayIpAddresses.contains(destinationAddress))) {
sendIcmpResponse(eth, inPort);
- // We remove the packet from the queue
- srManager.ipHandler.dequeuePacket(ipv4Packet, destinationAddress);
-
- // ICMP for any known host
- } else if (!srManager.hostService.getHostsByIp(destinationAddress).isEmpty()) {
- // TODO: known host packet should not be coming to controller - resend flows?
- srManager.ipHandler.forwardPackets(deviceId, destinationAddress);
-
- // ICMP for an unknown host in the subnet of the router
- } else if (config.inSameSubnet(deviceId, destinationAddress)) {
- srManager.arpHandler.sendArpRequest(deviceId, destinationAddress, inPort);
-
- // ICMP for an unknown host
} else {
- log.debug("ICMP request for unknown host {} ", destinationAddress);
- // We remove the packet from the queue
- srManager.ipHandler.dequeuePacket(ipv4Packet, destinationAddress);
+ log.trace("Ignore ICMP that targets for {}", destinationAddress);
}
+ // We remove the packet from the queue
+ srManager.ipHandler.dequeuePacket(ipv4Packet, destinationAddress);
}
/**
@@ -192,10 +176,8 @@
/**
* Process incoming ICMPv6 packet.
- * If it is an ICMP request to router or known host, then sends an ICMP response.
- * If it is an ICMP packet to known host and forward the packet to the host.
- * If it is an ICMP packet to unknown host in a subnet, then sends an ARP request
- * to the subnet.
+ * If it is an ICMPv6 request to router, then sends an ICMPv6 response.
+ * Otherwise ignore the packet.
*
* @param eth the incoming ICMPv6 packet
* @param inPort the input port
@@ -218,20 +200,8 @@
(destinationAddress.equals(routerIp.getIp6Address()) ||
gatewayIpAddresses.contains(destinationAddress))) {
sendIcmpv6Response(eth, inPort);
- // We remove the packet from the queue
- srManager.ipHandler.dequeuePacket(ipv6Packet, destinationAddress);
- // ICMP for any known host
- } else if (!srManager.hostService.getHostsByIp(destinationAddress).isEmpty()) {
- // TODO: known host packet should not be coming to controller - resend flows?
- srManager.ipHandler.forwardPackets(deviceId, destinationAddress);
- // ICMP for an unknown host in the subnet of the router
- } else if (config.inSameSubnet(deviceId, destinationAddress)) {
- sendNdpRequest(deviceId, destinationAddress, inPort);
- // ICMP for an unknown host or not configured host
} else {
- log.debug("ICMPv6 request for unknown host or not configured host {} ", destinationAddress);
- // We remove the packet from the queue
- srManager.ipHandler.dequeuePacket(ipv6Packet, destinationAddress);
+ log.trace("Ignore ICMPv6 that targets for {}", destinationAddress);
}
}
@@ -276,9 +246,7 @@
* @param hostService the host service
*/
public void processPacketIn(NeighbourMessageContext pkt, HostService hostService) {
- /*
- * First we validate the ndp packet
- */
+ // First we validate the ndp packet
SegmentRoutingAppConfig appConfig = srManager.cfgService
.getConfig(srManager.appId, SegmentRoutingAppConfig.class);
if (appConfig != null && appConfig.suppressSubnet().contains(pkt.inPort())) {
@@ -286,12 +254,6 @@
pkt.drop();
return;
}
- if (!validateSrcIp(pkt)) {
- log.debug("Ignore NDP packet discovered on {} with unexpected src ip address {}.",
- pkt.inPort(), pkt.sender());
- pkt.drop();
- return;
- }
if (pkt.type() == NeighbourMessageType.REQUEST) {
handleNdpRequest(pkt, hostService);
@@ -302,23 +264,6 @@
}
/**
- * Utility function to verify if the src ip belongs to the same
- * subnet configured on the port it is seen.
- *
- * @param pkt the ndp packet and context information
- * @return true if the src ip is a valid address for the subnet configured
- * for the connect point
- */
- private boolean validateSrcIp(NeighbourMessageContext pkt) {
- ConnectPoint connectPoint = pkt.inPort();
- IpPrefix subnet = config.getPortIPv6Subnet(
- connectPoint.deviceId(),
- connectPoint.port()
- );
- return subnet != null && subnet.contains(pkt.sender());
- }
-
- /**
* Helper method to handle the ndp requests.
*
* @param pkt the ndp packet request and context information
@@ -422,22 +367,15 @@
public void sendNdpRequest(DeviceId deviceId, IpAddress targetAddress, ConnectPoint inPort) {
byte[] senderMacAddress = new byte[MacAddress.MAC_ADDRESS_LENGTH];
byte[] senderIpAddress = new byte[Ip6Address.BYTE_LENGTH];
- /*
- * Retrieves device info.
- */
+ // Retrieves device info.
if (!getSenderInfo(senderMacAddress, senderIpAddress, deviceId, targetAddress)) {
log.warn("Aborting sendNdpRequest, we cannot get all the information needed");
return;
}
- /*
- * We have to compute the dst mac address and dst
- * ip address.
- */
+ // We have to compute the dst mac address and dst ip address.
byte[] dstIp = IPv6.getSolicitNodeAddress(targetAddress.toOctets());
byte[] dstMac = IPv6.getMCastMacAddress(dstIp);
- /*
- * Creates the request.
- */
+ // Creates the request.
Ethernet ndpRequest = NeighborSolicitation.buildNdpSolicit(
targetAddress.toOctets(),
senderIpAddress,