From e8600eb0c46ff9d6ddd628e08860c7c039cce9b7 Mon Sep 17 00:00:00 2001 From: Jonathan Hart Date: Mon, 12 Jan 2015 10:30:45 -0800 Subject: [PATCH] Handle packets that can't be deserialized properly. Catch BufferUnderflowExceptions that occur while trying to deserialzed Ethernet packet, and account for the fact that packets may not have been deserialized correctly when using InboundPackets. Addresses ONOS-605. Change-Id: Ia7191e62a339125c9c4d3fe0cf63f9c33eb74cb5 --- .../onosproject/fwd/ReactiveForwarding.java | 6 +++- .../ifwd/IntentReactiveForwarding.java | 10 +++++-- .../net/proxyarp/impl/ProxyArpManager.java | 14 ++++----- .../DefaultOpenFlowPacketContext.java | 9 ++++-- .../host/impl/HostLocationProvider.java | 30 +++++++++++-------- .../provider/lldp/impl/LinkDiscovery.java | 10 +++++-- 6 files changed, 50 insertions(+), 29 deletions(-) diff --git a/apps/fwd/src/main/java/org/onosproject/fwd/ReactiveForwarding.java b/apps/fwd/src/main/java/org/onosproject/fwd/ReactiveForwarding.java index fcf8efbdc3..06febb1485 100644 --- a/apps/fwd/src/main/java/org/onosproject/fwd/ReactiveForwarding.java +++ b/apps/fwd/src/main/java/org/onosproject/fwd/ReactiveForwarding.java @@ -27,6 +27,7 @@ import org.apache.felix.scr.annotations.Modified; import org.apache.felix.scr.annotations.Property; import org.apache.felix.scr.annotations.Reference; import org.apache.felix.scr.annotations.ReferenceCardinality; +import org.onlab.packet.Ethernet; import org.onosproject.core.ApplicationId; import org.onosproject.core.CoreService; import org.onosproject.net.Host; @@ -46,7 +47,6 @@ import org.onosproject.net.packet.PacketContext; import org.onosproject.net.packet.PacketProcessor; import org.onosproject.net.packet.PacketService; import org.onosproject.net.topology.TopologyService; -import org.onlab.packet.Ethernet; import org.osgi.service.component.ComponentContext; import org.slf4j.Logger; @@ -167,6 +167,10 @@ public class ReactiveForwarding { InboundPacket pkt = context.inPacket(); Ethernet ethPkt = pkt.parsed(); + if (ethPkt == null) { + return; + } + // Bail if this is deemed to be a control packet. if (isControlPacket(ethPkt)) { return; diff --git a/apps/ifwd/src/main/java/org/onosproject/ifwd/IntentReactiveForwarding.java b/apps/ifwd/src/main/java/org/onosproject/ifwd/IntentReactiveForwarding.java index 2db3df2add..5450bb4419 100644 --- a/apps/ifwd/src/main/java/org/onosproject/ifwd/IntentReactiveForwarding.java +++ b/apps/ifwd/src/main/java/org/onosproject/ifwd/IntentReactiveForwarding.java @@ -15,11 +15,14 @@ */ package org.onosproject.ifwd; +import static org.slf4j.LoggerFactory.getLogger; + import org.apache.felix.scr.annotations.Activate; import org.apache.felix.scr.annotations.Component; import org.apache.felix.scr.annotations.Deactivate; import org.apache.felix.scr.annotations.Reference; import org.apache.felix.scr.annotations.ReferenceCardinality; +import org.onlab.packet.Ethernet; import org.onosproject.core.ApplicationId; import org.onosproject.core.CoreService; import org.onosproject.net.Host; @@ -39,11 +42,8 @@ import org.onosproject.net.packet.PacketContext; import org.onosproject.net.packet.PacketProcessor; import org.onosproject.net.packet.PacketService; import org.onosproject.net.topology.TopologyService; -import org.onlab.packet.Ethernet; import org.slf4j.Logger; -import static org.slf4j.LoggerFactory.getLogger; - /** * WORK-IN-PROGRESS: Sample reactive forwarding application using intent framework. */ @@ -100,6 +100,10 @@ public class IntentReactiveForwarding { InboundPacket pkt = context.inPacket(); Ethernet ethPkt = pkt.parsed(); + if (ethPkt == null) { + return; + } + HostId srcId = HostId.hostId(ethPkt.getSourceMAC()); HostId dstId = HostId.hostId(ethPkt.getDestinationMAC()); diff --git a/core/net/src/main/java/org/onosproject/net/proxyarp/impl/ProxyArpManager.java b/core/net/src/main/java/org/onosproject/net/proxyarp/impl/ProxyArpManager.java index cd35359745..5a7dacd331 100644 --- a/core/net/src/main/java/org/onosproject/net/proxyarp/impl/ProxyArpManager.java +++ b/core/net/src/main/java/org/onosproject/net/proxyarp/impl/ProxyArpManager.java @@ -31,6 +31,12 @@ import org.apache.felix.scr.annotations.Deactivate; import org.apache.felix.scr.annotations.Reference; import org.apache.felix.scr.annotations.ReferenceCardinality; import org.apache.felix.scr.annotations.Service; +import org.onlab.packet.ARP; +import org.onlab.packet.Ethernet; +import org.onlab.packet.Ip4Address; +import org.onlab.packet.IpAddress; +import org.onlab.packet.MacAddress; +import org.onlab.packet.VlanId; import org.onosproject.core.ApplicationId; import org.onosproject.core.CoreService; import org.onosproject.net.ConnectPoint; @@ -61,12 +67,6 @@ import org.onosproject.net.packet.InboundPacket; import org.onosproject.net.packet.PacketContext; import org.onosproject.net.packet.PacketService; import org.onosproject.net.proxyarp.ProxyArpService; -import org.onlab.packet.ARP; -import org.onlab.packet.Ethernet; -import org.onlab.packet.Ip4Address; -import org.onlab.packet.IpAddress; -import org.onlab.packet.MacAddress; -import org.onlab.packet.VlanId; import org.slf4j.Logger; import com.google.common.collect.HashMultimap; @@ -309,7 +309,7 @@ public class ProxyArpManager implements ProxyArpService { public boolean handleArp(PacketContext context) { InboundPacket pkt = context.inPacket(); Ethernet ethPkt = pkt.parsed(); - if (ethPkt.getEtherType() == Ethernet.TYPE_ARP) { + if (ethPkt != null && ethPkt.getEtherType() == Ethernet.TYPE_ARP) { ARP arp = (ARP) ethPkt.getPayload(); if (arp.getOpCode() == ARP.OP_REPLY) { forward(ethPkt); diff --git a/openflow/api/src/main/java/org/onosproject/openflow/controller/DefaultOpenFlowPacketContext.java b/openflow/api/src/main/java/org/onosproject/openflow/controller/DefaultOpenFlowPacketContext.java index 2ea634000b..de1d6f05dd 100644 --- a/openflow/api/src/main/java/org/onosproject/openflow/controller/DefaultOpenFlowPacketContext.java +++ b/openflow/api/src/main/java/org/onosproject/openflow/controller/DefaultOpenFlowPacketContext.java @@ -16,6 +16,7 @@ package org.onosproject.openflow.controller; +import java.nio.BufferUnderflowException; import java.util.Collections; import java.util.concurrent.atomic.AtomicBoolean; @@ -85,8 +86,12 @@ public final class DefaultOpenFlowPacketContext implements OpenFlowPacketContext @Override public Ethernet parsed() { Ethernet eth = new Ethernet(); - eth.deserialize(pktin.getData(), 0, pktin.getTotalLen()); - return eth; + try { + eth.deserialize(pktin.getData(), 0, pktin.getData().length); + return eth; + } catch (BufferUnderflowException e) { + return null; + } } @Override diff --git a/providers/host/src/main/java/org/onosproject/provider/host/impl/HostLocationProvider.java b/providers/host/src/main/java/org/onosproject/provider/host/impl/HostLocationProvider.java index 7c3d2ecc32..0aa050d741 100644 --- a/providers/host/src/main/java/org/onosproject/provider/host/impl/HostLocationProvider.java +++ b/providers/host/src/main/java/org/onosproject/provider/host/impl/HostLocationProvider.java @@ -15,6 +15,11 @@ */ package org.onosproject.provider.host.impl; +import static org.slf4j.LoggerFactory.getLogger; + +import java.util.Dictionary; +import java.util.Set; + import org.apache.felix.scr.annotations.Activate; import org.apache.felix.scr.annotations.Component; import org.apache.felix.scr.annotations.Deactivate; @@ -22,6 +27,14 @@ import org.apache.felix.scr.annotations.Modified; import org.apache.felix.scr.annotations.Property; import org.apache.felix.scr.annotations.Reference; import org.apache.felix.scr.annotations.ReferenceCardinality; +import org.onlab.packet.ARP; +import org.onlab.packet.Ethernet; +import org.onlab.packet.IPacket; +import org.onlab.packet.IPv6; +import org.onlab.packet.IpAddress; +import org.onlab.packet.VlanId; +import org.onlab.packet.ndp.NeighborAdvertisement; +import org.onlab.packet.ndp.NeighborSolicitation; import org.onosproject.core.ApplicationId; import org.onosproject.core.CoreService; import org.onosproject.net.ConnectPoint; @@ -52,22 +65,9 @@ import org.onosproject.net.provider.AbstractProvider; import org.onosproject.net.provider.ProviderId; import org.onosproject.net.topology.Topology; import org.onosproject.net.topology.TopologyService; -import org.onlab.packet.ARP; -import org.onlab.packet.Ethernet; -import org.onlab.packet.IpAddress; -import org.onlab.packet.IPacket; -import org.onlab.packet.IPv6; -import org.onlab.packet.ndp.NeighborAdvertisement; -import org.onlab.packet.ndp.NeighborSolicitation; -import org.onlab.packet.VlanId; import org.osgi.service.component.ComponentContext; import org.slf4j.Logger; -import java.util.Dictionary; -import java.util.Set; - -import static org.slf4j.LoggerFactory.getLogger; - /** * Provider which uses an OpenFlow controller to detect network * end-station hosts. @@ -200,6 +200,10 @@ public class HostLocationProvider extends AbstractProvider implements HostProvid } Ethernet eth = context.inPacket().parsed(); + if (eth == null) { + return; + } + VlanId vlan = VlanId.vlanId(eth.getVlanID()); ConnectPoint heardOn = context.inPacket().receivedFrom(); diff --git a/providers/lldp/src/main/java/org/onosproject/provider/lldp/impl/LinkDiscovery.java b/providers/lldp/src/main/java/org/onosproject/provider/lldp/impl/LinkDiscovery.java index 863fbc01f5..85d1bec939 100644 --- a/providers/lldp/src/main/java/org/onosproject/provider/lldp/impl/LinkDiscovery.java +++ b/providers/lldp/src/main/java/org/onosproject/provider/lldp/impl/LinkDiscovery.java @@ -33,6 +33,9 @@ import java.util.concurrent.atomic.AtomicInteger; import org.jboss.netty.util.Timeout; import org.jboss.netty.util.TimerTask; +import org.onlab.packet.Ethernet; +import org.onlab.packet.ONOSLLDP; +import org.onlab.util.Timer; import org.onosproject.mastership.MastershipService; import org.onosproject.net.ConnectPoint; import org.onosproject.net.Device; @@ -47,9 +50,6 @@ import org.onosproject.net.packet.DefaultOutboundPacket; import org.onosproject.net.packet.OutboundPacket; import org.onosproject.net.packet.PacketContext; import org.onosproject.net.packet.PacketService; -import org.onlab.packet.Ethernet; -import org.onlab.packet.ONOSLLDP; -import org.onlab.util.Timer; import org.slf4j.Logger; // TODO: add 'fast discovery' mode: drop LLDPs in destination switch but listen for flow_removed messages @@ -208,6 +208,10 @@ public class LinkDiscovery implements TimerTask { */ public boolean handleLLDP(PacketContext context) { Ethernet eth = context.inPacket().parsed(); + if (eth == null) { + return false; + } + ONOSLLDP onoslldp = ONOSLLDP.parseONOSLLDP(eth); if (onoslldp != null) { final PortNumber dstPort =