Added restrictions for proxy ARP

core/api/src/main/java/org/onlab/onos/net/host/HostAdminService.java

@@ -1,7 +1,5 @@
 package org.onlab.onos.net.host;
 
-import java.util.Set;
-
 import org.onlab.onos.net.ConnectPoint;
 import org.onlab.onos.net.HostId;
 
@@ -47,20 +45,4 @@ public interface HostAdminService {
      */
     void clearAddresses(ConnectPoint connectPoint);
 
-    /**
-     * Returns the addresses information for all connection points.
-     *
-     * @return the set of address bindings for all connection points
-     */
-    Set<PortAddresses> getAddressBindings();
-
-    /**
-     * Retrieves the addresses that have been bound to the given connection
-     * point.
-     *
-     * @param connectPoint the connection point to retrieve address bindings
-     * for
-     * @return addresses bound to the port
-     */
-    PortAddresses getAddressBindingsForPort(ConnectPoint connectPoint);
 }

core/api/src/main/java/org/onlab/onos/net/host/HostService.java

@@ -109,6 +109,23 @@ public interface HostService {
      */
     void requestMac(IpAddress ip);
 
+    /**
+     * Returns the addresses information for all connection points.
+     *
+     * @return the set of address bindings for all connection points
+     */
+    Set<PortAddresses> getAddressBindings();
+
+    /**
+     * Retrieves the addresses that have been bound to the given connection
+     * point.
+     *
+     * @param connectPoint the connection point to retrieve address bindings
+     * for
+     * @return addresses bound to the port
+     */
+    PortAddresses getAddressBindingsForPort(ConnectPoint connectPoint);
+
     /**
      * Adds the specified host listener.
      *

core/api/src/main/java/org/onlab/onos/net/proxyarp/ProxyArpService.java

@@ -1,5 +1,6 @@
 package org.onlab.onos.net.proxyarp;
 
+import org.onlab.onos.net.ConnectPoint;
 import org.onlab.onos.net.packet.PacketContext;
 import org.onlab.packet.Ethernet;
 import org.onlab.packet.IpPrefix;
@@ -23,8 +24,9 @@ public interface ProxyArpService {
      * will be flooded at all edge ports.
      *
      * @param eth an arp request
+     * @param inPort the port the request was received on
      */
-    void reply(Ethernet eth);
+    void reply(Ethernet eth, ConnectPoint inPort);
 
     /**
      * Forwards an ARP request to its destination. Floods at the edge the ARP request if the

core/api/src/test/java/org/onlab/onos/net/host/HostServiceAdapter.java

@@ -75,4 +75,14 @@ public class HostServiceAdapter implements HostService {
     public void removeListener(HostListener listener) {
     }
 
+    @Override
+    public Set<PortAddresses> getAddressBindings() {
+        return null;
+    }
+
+    @Override
+    public PortAddresses getAddressBindingsForPort(ConnectPoint connectPoint) {
+        return null;
+    }
+
 }

core/net/src/main/java/org/onlab/onos/net/proxyarp/impl/ProxyArpManager.java

@@ -5,6 +5,7 @@ import static com.google.common.base.Preconditions.checkNotNull;
 import static org.slf4j.LoggerFactory.getLogger;
 
 import java.nio.ByteBuffer;
+import java.util.Collections;
 import java.util.List;
 import java.util.Map.Entry;
 import java.util.Set;
@@ -15,6 +16,7 @@ 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.onos.net.ConnectPoint;
 import org.onlab.onos.net.Device;
 import org.onlab.onos.net.Host;
 import org.onlab.onos.net.HostId;
@@ -27,6 +29,7 @@ import org.onlab.onos.net.device.DeviceService;
 import org.onlab.onos.net.flow.DefaultTrafficTreatment;
 import org.onlab.onos.net.flow.TrafficTreatment;
 import org.onlab.onos.net.host.HostService;
+import org.onlab.onos.net.host.PortAddresses;
 import org.onlab.onos.net.link.LinkEvent;
 import org.onlab.onos.net.link.LinkListener;
 import org.onlab.onos.net.link.LinkService;
@@ -37,7 +40,9 @@ import org.onlab.onos.net.packet.PacketService;
 import org.onlab.onos.net.proxyarp.ProxyArpService;
 import org.onlab.packet.ARP;
 import org.onlab.packet.Ethernet;
+import org.onlab.packet.IpAddress;
 import org.onlab.packet.IpPrefix;
+import org.onlab.packet.MacAddress;
 import org.onlab.packet.VlanId;
 import org.slf4j.Logger;
 
@@ -101,12 +106,46 @@ public class ProxyArpManager implements ProxyArpService {
     }
 
     @Override
-    public void reply(Ethernet eth) {
+    public void reply(Ethernet eth, ConnectPoint inPort) {
         checkNotNull(eth, REQUEST_NULL);
         checkArgument(eth.getEtherType() == Ethernet.TYPE_ARP,
                 REQUEST_NOT_ARP);
         ARP arp = (ARP) eth.getPayload();
         checkArgument(arp.getOpCode() == ARP.OP_REQUEST, NOT_ARP_REQUEST);
+        checkNotNull(inPort);
+
+        // If the source address matches one of our external addresses
+        // it could be a request from an internal host to an external
+        // address. Forward it over to the correct port.
+        IpAddress source = IpAddress.valueOf(arp.getSenderProtocolAddress());
+        PortAddresses sourceAddresses = findOutsidePortInSubnet(source);
+        if (sourceAddresses != null && !isOutsidePort(inPort)) {
+            for (IpPrefix subnet : sourceAddresses.ips()) {
+                if (subnet.toIpAddress().equals(source)) {
+                    sendTo(eth, sourceAddresses.connectPoint());
+                    return;
+                }
+            }
+        }
+
+        // If the request came from outside the network, only reply if it was
+        // for one of our external addresses.
+        if (isOutsidePort(inPort)) {
+            IpAddress target = IpAddress.valueOf(arp.getTargetProtocolAddress());
+            PortAddresses addresses = hostService.getAddressBindingsForPort(inPort);
+
+            for (IpPrefix interfaceAddress : addresses.ips()) {
+                if (interfaceAddress.toIpAddress().equals(target)) {
+                    Ethernet arpReply = buildArpReply(interfaceAddress,
+                            addresses.mac(), eth);
+                    sendTo(arpReply, inPort);
+                }
+            }
+
+            return;
+        }
+
+        // Continue with normal proxy ARP case
 
         VlanId vlan = VlanId.vlanId(eth.getVlanID());
         Set<Host> hosts = hostService.getHostsByIp(IpPrefix.valueOf(arp
@@ -128,12 +167,62 @@ public class ProxyArpManager implements ProxyArpService {
             return;
         }
 
-        Ethernet arpReply = buildArpReply(dst, eth);
+        Ethernet arpReply = buildArpReply(dst.ipAddresses().iterator().next(),
+                dst.mac(), eth);
         // TODO: check send status with host service.
+        sendTo(arpReply, src.location());
+    }
+
+    /**
+     * Outputs the given packet out the given port.
+     *
+     * @param packet the packet to send
+     * @param outPort the port to send it out
+     */
+    private void sendTo(Ethernet packet, ConnectPoint outPort) {
+        if (internalPorts.containsEntry(
+                deviceService.getDevice(outPort.deviceId()), outPort.port())) {
+            // Sanity check to make sure we don't send the packet out an
+            // internal port and create a loop (could happen due to
+            // misconfiguration).
+            return;
+        }
+
         TrafficTreatment.Builder builder = DefaultTrafficTreatment.builder();
-        builder.setOutput(src.location().port());
+        builder.setOutput(outPort.port());
-        packetService.emit(new DefaultOutboundPacket(src.location().deviceId(),
+        packetService.emit(new DefaultOutboundPacket(outPort.deviceId(),
-                builder.build(), ByteBuffer.wrap(arpReply.serialize())));
+                builder.build(), ByteBuffer.wrap(packet.serialize())));
+    }
+
+    /**
+     * Finds the port with an address in the subnet of the target address, if
+     * one exists.
+     *
+     * @param target the target address to find a matching external port for
+     * @return a PortAddresses object containing the external addresses if one
+     * was found, otherwise null.
+     */
+    private PortAddresses findOutsidePortInSubnet(IpAddress target) {
+        for (PortAddresses addresses : hostService.getAddressBindings()) {
+            for (IpPrefix prefix : addresses.ips()) {
+                if (prefix.contains(target)) {
+                    return new PortAddresses(addresses.connectPoint(),
+                            Collections.singleton(prefix), addresses.mac());
+                }
+            }
+        }
+        return null;
+    }
+
+    /**
+     * Returns whether the given port is an outside-facing port with an IP
+     * address configured.
+     *
+     * @param port the port to check
+     * @return true if the port is an outside-facing port, otherwise false
+     */
+    private boolean isOutsidePort(ConnectPoint port) {
+        return !hostService.getAddressBindingsForPort(port).ips().isEmpty();
     }
 
     @Override
@@ -167,7 +256,7 @@ public class ProxyArpManager implements ProxyArpService {
             if (arp.getOpCode() == ARP.OP_REPLY) {
                 forward(ethPkt);
             } else if (arp.getOpCode() == ARP.OP_REQUEST) {
-                reply(ethPkt);
+                reply(ethPkt, context.inPacket().receivedFrom());
             }
             context.block();
             return true;
@@ -185,12 +274,16 @@ public class ProxyArpManager implements ProxyArpService {
 
         synchronized (externalPorts) {
             for (Entry<Device, PortNumber> entry : externalPorts.entries()) {
+                ConnectPoint cp = new ConnectPoint(entry.getKey().id(), entry.getValue());
+                if (isOutsidePort(cp)) {
+                    continue;
+                }
+
                 builder = DefaultTrafficTreatment.builder();
                 builder.setOutput(entry.getValue());
                 packetService.emit(new DefaultOutboundPacket(entry.getKey().id(),
                         builder.build(), buf));
             }
-
         }
     }
 
@@ -234,15 +327,19 @@ public class ProxyArpManager implements ProxyArpService {
     }
 
     /**
-     * Builds an arp reply based on a request.
+     * Builds an ARP reply based on a request.
-     * @param h the host we want to send to
+     *
-     * @param request the arp request we got
+     * @param srcIp the IP address to use as the reply source
-     * @return an ethernet frame containing the arp reply
+     * @param srcMac the MAC address to use as the reply source
+     * @param request the ARP request we got
+     * @return an Ethernet frame containing the ARP reply
      */
-    private Ethernet buildArpReply(Host h, Ethernet request) {
+    private Ethernet buildArpReply(IpPrefix srcIp, MacAddress srcMac,
+            Ethernet request) {
+
         Ethernet eth = new Ethernet();
         eth.setDestinationMACAddress(request.getSourceMACAddress());
-        eth.setSourceMACAddress(h.mac().getAddress());
+        eth.setSourceMACAddress(srcMac.getAddress());
         eth.setEtherType(Ethernet.TYPE_ARP);
         eth.setVlanID(request.getVlanID());
 
@@ -253,12 +350,12 @@ public class ProxyArpManager implements ProxyArpService {
 
         arp.setProtocolAddressLength((byte) IpPrefix.INET_LEN);
         arp.setHardwareAddressLength((byte) Ethernet.DATALAYER_ADDRESS_LENGTH);
-        arp.setSenderHardwareAddress(h.mac().getAddress());
+        arp.setSenderHardwareAddress(srcMac.getAddress());
         arp.setTargetHardwareAddress(request.getSourceMACAddress());
 
         arp.setTargetProtocolAddress(((ARP) request.getPayload())
                 .getSenderProtocolAddress());
-        arp.setSenderProtocolAddress(h.ipAddresses().iterator().next().toRealInt());
+        arp.setSenderProtocolAddress(srcIp.toRealInt());
         eth.setPayload(arp);
         return eth;
     }

core/net/src/test/java/org/onlab/onos/net/proxyarp/impl/ProxyArpManagerTest.java

@@ -13,6 +13,7 @@ import java.util.Arrays;
 import java.util.Collections;
 import java.util.Comparator;
 import java.util.List;
+import java.util.Set;
 
 import org.junit.Before;
 import org.junit.Test;
@@ -31,6 +32,7 @@ import org.onlab.onos.net.device.DeviceService;
 import org.onlab.onos.net.flow.instructions.Instruction;
 import org.onlab.onos.net.flow.instructions.Instructions.OutputInstruction;
 import org.onlab.onos.net.host.HostService;
+import org.onlab.onos.net.host.PortAddresses;
 import org.onlab.onos.net.link.LinkListener;
 import org.onlab.onos.net.link.LinkService;
 import org.onlab.onos.net.packet.OutboundPacket;
@@ -50,12 +52,13 @@ import com.google.common.collect.Sets;
  */
 public class ProxyArpManagerTest {
 
-    private static final int NUM_DEVICES = 4;
+    private static final int NUM_DEVICES = 6;
     private static final int NUM_PORTS_PER_DEVICE = 3;
-    private static final int NUM_FLOOD_PORTS = 4;
+    private static final int NUM_ADDRESS_PORTS = NUM_DEVICES / 2;
+    private static final int NUM_FLOOD_PORTS = 3;
 
-    private static final IpPrefix IP1 = IpPrefix.valueOf("10.0.0.1/24");
+    private static final IpPrefix IP1 = IpPrefix.valueOf("192.168.1.1/24");
-    private static final IpPrefix IP2 = IpPrefix.valueOf("10.0.0.2/24");
+    private static final IpPrefix IP2 = IpPrefix.valueOf("192.168.1.2/24");
 
     private static final ProviderId PID = new ProviderId("of", "foo");
 
@@ -104,6 +107,9 @@ public class ProxyArpManagerTest {
      * The default topology is a unidirectional ring topology. Each switch has
      * 3 ports. Ports 2 and 3 have the links to neighbor switches, and port 1
      * is free (edge port).
+     * The first half of the switches have IP addresses configured on their
+     * free ports (port 1). The second half of the switches have no IP
+     * addresses configured.
      */
     private void createTopology() {
         deviceService = createMock(DeviceService.class);
@@ -114,6 +120,7 @@ public class ProxyArpManagerTest {
 
         createDevices(NUM_DEVICES, NUM_PORTS_PER_DEVICE);
         createLinks(NUM_DEVICES);
+        addAddressBindings();
     }
 
     /**
@@ -138,10 +145,11 @@ public class ProxyArpManagerTest {
                 ports.add(port);
             }
 
-            expect(deviceService.getPorts(devId)).andReturn(ports);
+            expect(deviceService.getPorts(devId)).andReturn(ports).anyTimes();
+            expect(deviceService.getDevice(devId)).andReturn(device).anyTimes();
         }
 
-        expect(deviceService.getDevices()).andReturn(devices);
+        expect(deviceService.getDevices()).andReturn(devices).anyTimes();
         replay(deviceService);
     }
 
@@ -173,6 +181,31 @@ public class ProxyArpManagerTest {
         replay(linkService);
     }
 
+    private void addAddressBindings() {
+        Set<PortAddresses> addresses = Sets.newHashSet();
+
+        for (int i = 1; i <= NUM_ADDRESS_PORTS; i++) {
+            ConnectPoint cp = new ConnectPoint(getDeviceId(i), P1);
+            IpPrefix prefix1 = IpPrefix.valueOf("10.0." + (2 * i - 1) + ".1/24");
+            IpPrefix prefix2 = IpPrefix.valueOf("10.0." + (2 * i) + ".1/24");
+            PortAddresses pa = new PortAddresses(cp,
+                    Sets.newHashSet(prefix1, prefix2), MacAddress.valueOf(i));
+            addresses.add(pa);
+
+            expect(hostService.getAddressBindingsForPort(cp))
+                    .andReturn(pa).anyTimes();
+        }
+
+        expect(hostService.getAddressBindings()).andReturn(addresses).anyTimes();
+
+        for (int i = 1; i <= NUM_FLOOD_PORTS; i++) {
+            ConnectPoint cp = new ConnectPoint(getDeviceId(i + NUM_ADDRESS_PORTS),
+                    P1);
+            expect(hostService.getAddressBindingsForPort(cp))
+                    .andReturn(new PortAddresses(cp, null, null)).anyTimes();
+        }
+    }
+
     /**
      * Tests {@link ProxyArpManager#known(IpPrefix)} in the case where the
      * IP address is not known.
@@ -210,10 +243,10 @@ public class ProxyArpManagerTest {
      */
     @Test
     public void testReplyKnown() {
-        Host replyer = new DefaultHost(PID, HID1, MAC1, VLAN1, LOC2,
+        Host replyer = new DefaultHost(PID, HID1, MAC1, VLAN1, getLocation(4),
                 Collections.singleton(IP1));
 
-        Host requestor = new DefaultHost(PID, HID2, MAC2, VLAN1, LOC1,
+        Host requestor = new DefaultHost(PID, HID2, MAC2, VLAN1, getLocation(5),
                 Collections.singleton(IP2));
 
         expect(hostService.getHostsByIp(IpPrefix.valueOf(IP1.toOctets())))
@@ -224,11 +257,11 @@ public class ProxyArpManagerTest {
 
         Ethernet arpRequest = buildArp(ARP.OP_REQUEST, MAC2, null, IP2, IP1);
 
-        proxyArp.reply(arpRequest);
+        proxyArp.reply(arpRequest, getLocation(5));
 
         assertEquals(1, packetService.packets.size());
         Ethernet arpReply = buildArp(ARP.OP_REPLY, MAC1, MAC2, IP1, IP2);
-        verifyPacketOut(arpReply, LOC1, packetService.packets.get(0));
+        verifyPacketOut(arpReply, getLocation(5), packetService.packets.get(0));
     }
 
     /**
@@ -238,7 +271,7 @@ public class ProxyArpManagerTest {
      */
     @Test
     public void testReplyUnknown() {
-        Host requestor = new DefaultHost(PID, HID2, MAC2, VLAN1, LOC1,
+        Host requestor = new DefaultHost(PID, HID2, MAC2, VLAN1, getLocation(5),
                 Collections.singleton(IP2));
 
         expect(hostService.getHostsByIp(IpPrefix.valueOf(IP1.toOctets())))
@@ -249,7 +282,7 @@ public class ProxyArpManagerTest {
 
         Ethernet arpRequest = buildArp(ARP.OP_REQUEST, MAC2, null, IP2, IP1);
 
-        proxyArp.reply(arpRequest);
+        proxyArp.reply(arpRequest, getLocation(5));
 
         verifyFlood(arpRequest);
     }
@@ -262,10 +295,10 @@ public class ProxyArpManagerTest {
      */
     @Test
     public void testReplyDifferentVlan() {
-        Host replyer = new DefaultHost(PID, HID1, MAC1, VLAN2, LOC2,
+        Host replyer = new DefaultHost(PID, HID1, MAC1, VLAN2, getLocation(4),
                 Collections.singleton(IP1));
 
-        Host requestor = new DefaultHost(PID, HID2, MAC2, VLAN1, LOC1,
+        Host requestor = new DefaultHost(PID, HID2, MAC2, VLAN1, getLocation(5),
                 Collections.singleton(IP2));
 
         expect(hostService.getHostsByIp(IpPrefix.valueOf(IP1.toOctets())))
@@ -276,11 +309,84 @@ public class ProxyArpManagerTest {
 
         Ethernet arpRequest = buildArp(ARP.OP_REQUEST, MAC2, null, IP2, IP1);
 
-        proxyArp.reply(arpRequest);
+        proxyArp.reply(arpRequest, getLocation(5));
 
         verifyFlood(arpRequest);
     }
 
+    @Test
+    public void testReplyToRequestForUs() {
+        IpPrefix theirIp = IpPrefix.valueOf("10.0.1.254/24");
+        IpPrefix ourFirstIp = IpPrefix.valueOf("10.0.1.1/24");
+        IpPrefix ourSecondIp = IpPrefix.valueOf("10.0.2.1/24");
+        MacAddress ourMac = MacAddress.valueOf(1L);
+
+        Host requestor = new DefaultHost(PID, HID2, MAC2, VLAN1, LOC1,
+                Collections.singleton(theirIp));
+
+        expect(hostService.getHost(HID2)).andReturn(requestor);
+        replay(hostService);
+
+        Ethernet arpRequest = buildArp(ARP.OP_REQUEST, MAC2, null, theirIp, ourFirstIp);
+
+        proxyArp.reply(arpRequest, LOC1);
+
+        assertEquals(1, packetService.packets.size());
+        Ethernet arpReply = buildArp(ARP.OP_REPLY, ourMac, MAC2, ourFirstIp, theirIp);
+        verifyPacketOut(arpReply, LOC1, packetService.packets.get(0));
+
+        // Test a request for the second address on that port
+        packetService.packets.clear();
+        arpRequest = buildArp(ARP.OP_REQUEST, MAC2, null, theirIp, ourSecondIp);
+
+        proxyArp.reply(arpRequest, LOC1);
+
+        assertEquals(1, packetService.packets.size());
+        arpReply = buildArp(ARP.OP_REPLY, ourMac, MAC2, ourSecondIp, theirIp);
+        verifyPacketOut(arpReply, LOC1, packetService.packets.get(0));
+    }
+
+    @Test
+    public void testReplyExternalPortBadRequest() {
+        replay(hostService); // no further host service expectations
+
+        IpPrefix theirIp = IpPrefix.valueOf("10.0.1.254/24");
+
+        // Request for a valid external IP address but coming in the wrong port
+        Ethernet arpRequest = buildArp(ARP.OP_REQUEST, MAC1, null, theirIp,
+                IpPrefix.valueOf("10.0.3.1"));
+        proxyArp.reply(arpRequest, LOC1);
+        assertEquals(0, packetService.packets.size());
+
+        // Request for a valid internal IP address but coming in an external port
+        packetService.packets.clear();
+        arpRequest = buildArp(ARP.OP_REQUEST, MAC1, null, theirIp, IP1);
+        proxyArp.reply(arpRequest, LOC1);
+        assertEquals(0, packetService.packets.size());
+    }
+
+    @Test
+    public void testReplyToRequestFromUs() {
+        replay(hostService); // no further host service expectations
+
+        IpPrefix ourIp = IpPrefix.valueOf("10.0.1.1/24");
+        MacAddress ourMac = MacAddress.valueOf(1L);
+        IpPrefix theirIp = IpPrefix.valueOf("10.0.1.100/24");
+
+        // This is a request from something inside our network (like a BGP
+        // daemon) to an external host.
+        Ethernet arpRequest = buildArp(ARP.OP_REQUEST, ourMac, null, ourIp, theirIp);
+        proxyArp.reply(arpRequest, getLocation(5));
+
+        assertEquals(1, packetService.packets.size());
+        verifyPacketOut(arpRequest, getLocation(1), packetService.packets.get(0));
+
+        // The same request from a random external port should fail
+        packetService.packets.clear();
+        proxyArp.reply(arpRequest, getLocation(2));
+        assertEquals(0, packetService.packets.size());
+    }
+
     /**
      * Tests {@link ProxyArpManager#forward(Ethernet)} in the case where the
      * destination host is known.
@@ -338,7 +444,8 @@ public class ProxyArpManagerTest {
             });
 
         for (int i = 0; i < NUM_FLOOD_PORTS; i++) {
-            ConnectPoint cp = new ConnectPoint(getDeviceId(i + 1), PortNumber.portNumber(1));
+            ConnectPoint cp = new ConnectPoint(getDeviceId(NUM_ADDRESS_PORTS + i + 1),
+                    PortNumber.portNumber(1));
 
             OutboundPacket outboundPacket = packetService.packets.get(i);
             verifyPacketOut(packet, cp, outboundPacket);
@@ -372,6 +479,10 @@ public class ProxyArpManagerTest {
         return DeviceId.deviceId("" + i);
     }
 
+    private static HostLocation getLocation(int i) {
+        return new HostLocation(new ConnectPoint(getDeviceId(i), P1), 123L);
+    }
+
     /**
      * Builds an ARP packet with the given parameters.
      *

utils/misc/src/test/java/org/onlab/packet/IpPrefixTest.java

@@ -108,5 +108,10 @@ public class IpPrefixTest {
         IpAddress addr = IpAddress.valueOf("192.168.10.1");
 
         assertTrue(intf.contains(addr));
+
+        IpPrefix intf1 = IpPrefix.valueOf("10.0.0.101/24");
+        IpAddress addr1 = IpAddress.valueOf("10.0.0.4");
+
+        assertTrue(intf1.contains(addr1));
     }
 }