From 6fe222b2d5c6f52920fd4794083460796fad2bd4 Mon Sep 17 00:00:00 2001
From: Kanzhe Jiang <kanzhe.jiang@bigswitch.com>
Date: Thu, 15 Mar 2012 16:32:51 -0700
Subject: [PATCH] allow MAC-IP learning from both ARP request and reply

---
 .../internal/DeviceManagerImpl.java           | 18 +++---
 .../internal/DeviceManagerImplTest.java       | 58 -------------------
 2 files changed, 11 insertions(+), 65 deletions(-)

diff --git a/src/main/java/net/floodlightcontroller/devicemanager/internal/DeviceManagerImpl.java b/src/main/java/net/floodlightcontroller/devicemanager/internal/DeviceManagerImpl.java
index 43e484853..379d704e0 100755
--- a/src/main/java/net/floodlightcontroller/devicemanager/internal/DeviceManagerImpl.java
+++ b/src/main/java/net/floodlightcontroller/devicemanager/internal/DeviceManagerImpl.java
@@ -875,13 +875,17 @@ public class DeviceManagerImpl implements IDeviceManagerService, IOFMessageListe
                 if (networkAddress != null) {
                     updateNetworkAddressLastSeen = true;
                 } else if (eth != null && (eth.getPayload() instanceof ARP)) {
-                    ARP arp = (ARP)eth.getPayload();
-                    // Only learn new MAC-IP mapping on ARP reply
-                    if (arp.getOpCode() == 0x02) {
-                        networkAddress = new DeviceNetworkAddress(nwSrc, 
-                                                                currentDate);
-                        newNetworkAddress = true;
-                    }
+                	/** MAC-IP association should be learnt from both ARP request and reply.
+                	 *  Since a host learns some other host's mac to ip mapping after receiving 
+                	 *  an ARP request from the host.
+                	 *  
+                	 *  However, device's MAC-IP mapping could be wrong if a host sends a ARP request with
+                	 *  an IP other than its own. Unfortunately, there isn't an easy way to allow both learning
+                	 *  and prevent incorrect learning.
+                	 */
+                    networkAddress = new DeviceNetworkAddress(nwSrc, 
+                                                            currentDate);
+                    newNetworkAddress = true;
                 }
 
                 // Also, if this address is currently mapped to a different 
diff --git a/src/test/java/net/floodlightcontroller/devicemanager/internal/DeviceManagerImplTest.java b/src/test/java/net/floodlightcontroller/devicemanager/internal/DeviceManagerImplTest.java
index 99b264169..8b401354c 100644
--- a/src/test/java/net/floodlightcontroller/devicemanager/internal/DeviceManagerImplTest.java
+++ b/src/test/java/net/floodlightcontroller/devicemanager/internal/DeviceManagerImplTest.java
@@ -307,64 +307,6 @@ public class DeviceManagerImplTest extends FloodlightTestCase {
         deviceManager.clearAllDeviceStateFromMemory();
     }
     
-    @Test
-    public void testDeviceDiscover_NegTest() throws Exception {
-        
-        byte[] dataLayerSource = ((Ethernet)this.testARPReqPacket_1).getSourceMACAddress();
-
-        // Mock up our expected behavior
-        IOFSwitch mockSwitch = createMock(IOFSwitch.class);
-        expect(mockSwitch.getId()).andReturn(1L).anyTimes();
-        expect(mockSwitch.getStringId()).andReturn("00:00:00:00:00:00:00:01").anyTimes();
-        ITopologyService mockTopology = createMock(ITopologyService.class);
-        expect(mockTopology.isInternal(1L, (short)1)).andReturn(false);
-        deviceManager.setTopology(mockTopology);
-        
-        Date currentDate = new Date();
-        
-        // build our expected Device
-        Device device = new Device();
-        device.setDataLayerAddress(dataLayerSource);
-        device.addAttachmentPoint(new SwitchPortTuple(mockSwitch, (short)1), currentDate);
-        Integer ipaddr = IPv4.toIPv4Address("192.168.1.4");
-        device.addNetworkAddress(ipaddr, currentDate);
-        
-        // Start recording the replay on the mocks
-        replay(mockSwitch, mockTopology);
-        // Get the listener and trigger the packet in
-        mockFloodlightProvider.dispatchMessage(mockSwitch, this.packetIn_4);
-
-        // Verify the replay matched our expectations
-        verify(mockSwitch, mockTopology);
-
-        // Verify the device
-        Device rdevice = deviceManager.getDeviceByDataLayerAddress(dataLayerSource);
-        assertEquals(device, rdevice);
-
-        reset(mockSwitch, mockTopology);
-        expect(mockSwitch.getId()).andReturn(1L).anyTimes();
-        expect(mockSwitch.getStringId()).andReturn("00:00:00:00:00:00:00:01").anyTimes();
-        expect(mockTopology.isInternal(1L, (short)1)).andReturn(false);
-
-        // The device is learnt from the first packetIn with one network address
-        assertEquals(1, deviceManager.getDeviceByIPv4Address(ipaddr).getNetworkAddresses().size());
-        
-        // Start recording the replay on the mocks
-        replay(mockSwitch, mockTopology);
-        // Get the listener and trigger the packet in
-        mockFloodlightProvider.dispatchMessage(mockSwitch, this.packetIn_5);
-
-        // Verify the replay matched our expectations
-        verify(mockSwitch, mockTopology);
-
-        // The new network address should not be learnt from ARP request.
-        assertNull(deviceManager.getDeviceByIPv4Address(IPv4.toIPv4Address("192.168.1.14")));
-        assertEquals(1, deviceManager.getDeviceByDataLayerAddress(dataLayerSource).getNetworkAddresses().size());
-        
-        // Reset the device cache
-        deviceManager.clearAllDeviceStateFromMemory();
-    }
-    
     @Test
     public void testDeviceRecoverFromStorage() throws Exception {
         byte[] dataLayerSource = ((Ethernet)this.testARPReplyPacket_2).getSourceMACAddress();
-- 
GitLab