[CORD-1735] Add "relayAgentIps" option to DHCP relay application config

Change-Id: I2d95b5a285c81c15002ad94686b26ce03910198e

apps/dhcprelay/src/main/java/org/onosproject/dhcprelay/Dhcp4HandlerImpl.java

@@ -117,6 +117,7 @@ public class Dhcp4HandlerImpl implements DhcpHandler {
     private MacAddress dhcpConnectMac = null;
     private VlanId dhcpConnectVlan = null;
     private Ip4Address dhcpGatewayIp = null;
+    private Ip4Address relayAgentIp = null;
 
     @Activate
     protected void activate() {
@@ -128,6 +129,12 @@ public class Dhcp4HandlerImpl implements DhcpHandler {
         hostService.removeListener(hostListener);
         this.dhcpConnectMac = null;
         this.dhcpConnectVlan = null;
+
+        if (dhcpGatewayIp != null) {
+            hostService.stopMonitoringIp(dhcpGatewayIp);
+        } else if (dhcpServerIp != null) {
+            hostService.stopMonitoringIp(dhcpServerIp);
+        }
     }
 
     @Override
@@ -231,6 +238,8 @@ public class Dhcp4HandlerImpl implements DhcpHandler {
             this.dhcpConnectVlan = host.vlan();
             this.dhcpConnectMac = host.mac();
         }
+
+        this.relayAgentIp = serverConfig.getRelayAgentIp4().orElse(null);
     }
 
     @Override
@@ -238,6 +247,7 @@ public class Dhcp4HandlerImpl implements DhcpHandler {
         log.warn("Indirect config feature for DHCPv4 handler not implement yet");
     }
 
+    @Override
     public void processDhcpPacket(PacketContext context, BasePacket payload) {
         checkNotNull(payload, "DHCP payload can't be null");
         checkState(payload instanceof DHCP, "Payload is not a DHCP");
@@ -340,7 +350,7 @@ public class Dhcp4HandlerImpl implements DhcpHandler {
      * @return the first interface IP; null if not exists an IP address in
      *         these interfaces
      */
-    private Ip4Address getRelayAgentIPv4Address(Interface iface) {
+    private Ip4Address getFirstIpFromInterface(Interface iface) {
         checkNotNull(iface, "Interface can't be null");
         return iface.ipAddressesList().stream()
                 .map(InterfaceIpAddress::ipAddress)
@@ -398,9 +408,9 @@ public class Dhcp4HandlerImpl implements DhcpHandler {
             log.warn("Can't get server interface, ignore");
             return null;
         }
-        Ip4Address relayAgentIp = getRelayAgentIPv4Address(serverInterface);
+        Ip4Address ipFacingServer = getFirstIpFromInterface(serverInterface);
-        MacAddress relayAgentMac = serverInterface.mac();
+        MacAddress macFacingServer = serverInterface.mac();
-        if (relayAgentIp == null || relayAgentMac == null) {
+        if (ipFacingServer == null || macFacingServer == null) {
             log.warn("No IP address for server Interface {}", serverInterface);
             return null;
         }
@@ -414,11 +424,11 @@ public class Dhcp4HandlerImpl implements DhcpHandler {
         }
         // get dhcp header.
         Ethernet etherReply = (Ethernet) ethernetPacket.clone();
-        etherReply.setSourceMACAddress(relayAgentMac);
+        etherReply.setSourceMACAddress(macFacingServer);
         etherReply.setDestinationMACAddress(dhcpConnectMac);
         etherReply.setVlanID(dhcpConnectVlan.toShort());
         IPv4 ipv4Packet = (IPv4) etherReply.getPayload();
-        ipv4Packet.setSourceAddress(relayAgentIp.toInt());
+        ipv4Packet.setSourceAddress(ipFacingServer.toInt());
         ipv4Packet.setDestinationAddress(dhcpServerIp.toInt());
         UDP udpPacket = (UDP) ipv4Packet.getPayload();
         DHCP dhcpPacket = (DHCP) udpPacket.getPayload();
@@ -459,6 +469,12 @@ public class Dhcp4HandlerImpl implements DhcpHandler {
             dhcpPacket.setGatewayIPAddress(clientInterfaceIp.toInt());
         }
 
+        // replace giaddr if relay agent IP is set
+        // FIXME for both direct and indirect case now, should be separated
+        if (relayAgentIp != null) {
+            dhcpPacket.setGatewayIPAddress(relayAgentIp.toInt());
+        }
+
         udpPacket.setPayload(dhcpPacket);
         // As a DHCP relay, the source port should be server port(67) instead
         // of client port(68)
@@ -590,8 +606,8 @@ public class Dhcp4HandlerImpl implements DhcpHandler {
 
         // we leave the srcMac from the original packet
         // figure out the relay agent IP corresponding to the original request
-        Ip4Address relayAgentIP = getRelayAgentIPv4Address(clientInterface);
+        Ip4Address ipFacingClient = getFirstIpFromInterface(clientInterface);
-        if (relayAgentIP == null) {
+        if (ipFacingClient == null) {
             log.warn("Cannot determine relay agent interface Ipv4 addr for host {}/{}. "
                              + "Aborting relay for dhcp packet from server {}",
                      etherReply.getDestinationMAC(), clientInterface.vlan(),
@@ -600,7 +616,7 @@ public class Dhcp4HandlerImpl implements DhcpHandler {
         }
         // SRC_IP: relay agent IP
         // DST_IP: offered IP
-        ipv4Packet.setSourceAddress(relayAgentIP.toInt());
+        ipv4Packet.setSourceAddress(ipFacingClient.toInt());
         ipv4Packet.setDestinationAddress(dhcpPayload.getYourIPAddress());
         udpPacket.setSourcePort(UDP.DHCP_SERVER_PORT);
         if (directlyConnected(dhcpPayload)) {

apps/dhcprelay/src/main/java/org/onosproject/dhcprelay/DhcpRelayManager.java

@@ -181,9 +181,6 @@ public class DhcpRelayManager implements DhcpRelayService {
         packetService.removeProcessor(dhcpRelayPacketProcessor);
         cancelDhcpPackets();
         cancelArpPackets();
-        v4Handler.getDhcpGatewayIp().ifPresent(hostService::stopMonitoringIp);
-        v4Handler.getDhcpServerIp().ifPresent(hostService::stopMonitoringIp);
-        // TODO: DHCPv6 Handler
 
         compCfgService.unregisterProperties(getClass(), false);
         log.info("DHCP-RELAY Stopped");
@@ -236,15 +233,15 @@ public class DhcpRelayManager implements DhcpRelayService {
             // Ignore if config is not present
             return;
         }
-        if (config instanceof DefaultDhcpRelayConfig) {
+
-            DefaultDhcpRelayConfig defaultConfig = (DefaultDhcpRelayConfig) config;
-            v4Handler.setDefaultDhcpServerConfigs(defaultConfig.dhcpServerConfigs());
-            v6Handler.setDefaultDhcpServerConfigs(defaultConfig.dhcpServerConfigs());
-        }
         if (config instanceof IndirectDhcpRelayConfig) {
             IndirectDhcpRelayConfig indirectConfig = (IndirectDhcpRelayConfig) config;
             v4Handler.setIndirectDhcpServerConfigs(indirectConfig.dhcpServerConfigs());
             v6Handler.setIndirectDhcpServerConfigs(indirectConfig.dhcpServerConfigs());
+        } else if (config instanceof DefaultDhcpRelayConfig) {
+            DefaultDhcpRelayConfig defaultConfig = (DefaultDhcpRelayConfig) config;
+            v4Handler.setDefaultDhcpServerConfigs(defaultConfig.dhcpServerConfigs());
+            v6Handler.setDefaultDhcpServerConfigs(defaultConfig.dhcpServerConfigs());
         }
     }
 

apps/dhcprelay/src/main/java/org/onosproject/dhcprelay/config/DefaultDhcpRelayConfig.java

@@ -29,8 +29,6 @@ import java.util.concurrent.atomic.AtomicBoolean;
 public class DefaultDhcpRelayConfig extends Config<ApplicationId> {
     public static final String KEY = "default";
 
-
-
     @Override
     public boolean isValid() {
         // check if all configs are valid

apps/dhcprelay/src/main/java/org/onosproject/dhcprelay/config/DhcpServerConfig.java

@@ -33,12 +33,15 @@ public class DhcpServerConfig {
     private static final String DHCP_CONNECT_POINT = "dhcpServerConnectPoint";
     private static final String DHCP_SERVER_IP = "serverIps";
     private static final String DHCP_GATEWAY_IP = "gatewayIps";
+    private static final String RELAY_AGENT_IP = "relayAgentIps";
 
     private ConnectPoint connectPoint;
     private Ip4Address serverIp4Addr;
     private Ip4Address gatewayIp4Addr;
+    private Ip4Address relayAgentIp4Addr;
     private Ip6Address serverIp6Addr;
     private Ip6Address gatewayIp6Addr;
+    private Ip6Address relayAgentIp6Addr;
 
     protected DhcpServerConfig() {
         // empty config not allowed here
@@ -68,22 +71,34 @@ public class DhcpServerConfig {
             }
         });
 
-        if (!config.has(DHCP_GATEWAY_IP)) {
+        if (config.has(DHCP_GATEWAY_IP)) {
-            // gateway ip doesn't exist, ignore the gateway
+            ArrayNode gatewayIps = (ArrayNode) config.path(DHCP_GATEWAY_IP);
-            return;
+            gatewayIps.forEach(node -> {
+                if (node.isTextual()) {
+                    IpAddress ip = IpAddress.valueOf(node.asText());
+                    if (ip.isIp4() && gatewayIp4Addr == null) {
+                        gatewayIp4Addr = ip.getIp4Address();
+                    }
+                    if (ip.isIp6() && gatewayIp6Addr == null) {
+                        gatewayIp6Addr = ip.getIp6Address();
+                    }
+                }
+            });
         }
-        ArrayNode gatewayIps = (ArrayNode) config.path(DHCP_GATEWAY_IP);
+        if (config.has(RELAY_AGENT_IP)) {
-        gatewayIps.forEach(node -> {
+            ArrayNode relayAgentIps = (ArrayNode) config.path(RELAY_AGENT_IP);
-            if (node.isTextual()) {
+            relayAgentIps.forEach(node -> {
-                IpAddress ip = IpAddress.valueOf(node.asText());
+                if (node.isTextual()) {
-                if (ip.isIp4() && gatewayIp4Addr == null) {
+                    IpAddress ip = IpAddress.valueOf(node.asText());
-                    gatewayIp4Addr = ip.getIp4Address();
+                    if (ip.isIp4() && relayAgentIp4Addr == null) {
+                        relayAgentIp4Addr = ip.getIp4Address();
+                    }
+                    if (ip.isIp6() && relayAgentIp6Addr == null) {
+                        relayAgentIp6Addr = ip.getIp6Address();
+                    }
                 }
-                if (ip.isIp6() && gatewayIp6Addr == null) {
+            });
-                    gatewayIp6Addr = ip.getIp6Address();
+        }
-                }
-            }
-        });
     }
 
     /**
@@ -146,4 +161,26 @@ public class DhcpServerConfig {
     public Optional<Ip6Address> getDhcpGatewayIp6() {
         return Optional.ofNullable(gatewayIp6Addr);
     }
+
+    /**
+     * Returns the optional IPv4 address for relay agent, if configured.
+     * This option is used if we want to replace the giaddr field in DHCPv4
+     * payload.
+     *
+     * @return the giaddr; empty value if not set
+     */
+    public Optional<Ip4Address> getRelayAgentIp4() {
+        return Optional.ofNullable(relayAgentIp4Addr);
+    }
+
+    /**
+     * Returns the optional IPv6 address for relay agent, if configured.
+     * This option is used if we want to replace the link-address field in DHCPv6
+     * payload.
+     *
+     * @return the giaddr; empty value if not set
+     */
+    public Optional<Ip6Address> getRelayAgentIp6() {
+        return Optional.ofNullable(relayAgentIp6Addr);
+    }
 }

apps/dhcprelay/src/test/java/org/onosproject/dhcprelay/DhcpRelayManagerTest.java

@@ -27,6 +27,7 @@ import org.junit.Before;
 import org.junit.Test;
 import org.onlab.packet.ARP;
 import org.onlab.packet.DHCP;
+import org.onlab.packet.DeserializationException;
 import org.onlab.packet.Ethernet;
 import org.onlab.packet.IPv4;
 import org.onlab.packet.Ip4Address;
@@ -152,6 +153,9 @@ public class DhcpRelayManagerTest {
                                                                     SERVER_IFACE_MAC,
                                                                     SERVER_VLAN);
 
+    // Relay agent config
+    private static final Ip4Address RELAY_AGENT_IP = Ip4Address.valueOf("10.0.4.254");
+
     // Components
     private static final ApplicationId APP_ID = TestApplicationId.create(DhcpRelayManager.DHCP_RELAY_APP);
     private static final DefaultDhcpRelayConfig CONFIG = new MockDefaultDhcpRelayConfig();
@@ -187,8 +191,8 @@ public class DhcpRelayManagerTest {
                 .andReturn(APP_ID).anyTimes();
 
         manager.hostService = createNiceMock(HostService.class);
-        expect(manager.hostService.getHostsByIp(anyObject())).andReturn(ImmutableSet.of(SERVER_HOST));
+        expect(manager.hostService.getHostsByIp(anyObject())).andReturn(ImmutableSet.of(SERVER_HOST)).anyTimes();
-        expect(manager.hostService.getHost(OUTER_RELAY_HOST_ID)).andReturn(OUTER_RELAY_HOST);
+        expect(manager.hostService.getHost(OUTER_RELAY_HOST_ID)).andReturn(OUTER_RELAY_HOST).anyTimes();
 
         packetService = new MockPacketService();
         manager.packetService = packetService;
@@ -299,6 +303,24 @@ public class DhcpRelayManagerTest {
         assertEquals(Route.Source.STATIC, route.source());
     }
 
+    @Test
+    public void testWithRelayAgentConfig() throws DeserializationException {
+        manager.v4Handler
+                .setDefaultDhcpServerConfigs(ImmutableList.of(new MockDhcpServerConfig(RELAY_AGENT_IP)));
+        packetService.processPacket(new TestDhcpRequestPacketContext(CLIENT2_MAC,
+                                                                     CLIENT2_VLAN,
+                                                                     CLIENT2_CP,
+                                                                     INTERFACE_IP.ipAddress().getIp4Address(),
+                                                                     true));
+        OutboundPacket outPacket = packetService.emittedPacket;
+        byte[] outData = outPacket.data().array();
+        Ethernet eth = Ethernet.deserializer().deserialize(outData, 0, outData.length);
+        IPv4 ip = (IPv4) eth.getPayload();
+        UDP udp = (UDP) ip.getPayload();
+        DHCP dhcp = (DHCP) udp.getPayload();
+        assertEquals(RELAY_AGENT_IP.toInt(), dhcp.getGatewayIPAddress());
+    }
+
     @Test
     public void testArpRequest() throws Exception {
         packetService.processPacket(new TestArpRequestPacketContext(CLIENT_INTERFACE));
@@ -319,11 +341,27 @@ public class DhcpRelayManagerTest {
 
         @Override
         public List<DhcpServerConfig> dhcpServerConfigs() {
-            return ImmutableList.of(new MockDhcpServerConfig());
+            return ImmutableList.of(new MockDhcpServerConfig(null));
         }
     }
 
     private static class MockDhcpServerConfig extends DhcpServerConfig {
+        Ip4Address relayAgentIp;
+
+        /**
+         * Create mocked version DHCP server config.
+         *
+         * @param relayAgentIp the relay agent Ip config; null if we don't need it
+         */
+        public MockDhcpServerConfig(Ip4Address relayAgentIp) {
+            this.relayAgentIp = relayAgentIp;
+        }
+
+        @Override
+        public Optional<Ip4Address> getRelayAgentIp4() {
+            return Optional.ofNullable(relayAgentIp);
+        }
+
         @Override
         public Optional<ConnectPoint> getDhcpServerConnectPoint() {
             return Optional.of(SERVER_CONNECT_POINT);

apps/dhcprelay/src/test/resources/dhcp-relay.json

@@ -5,13 +5,15 @@
         {
           "dhcpServerConnectPoint": "of:0000000000000002/2",
           "serverIps": ["172.168.10.2", "2000::200:1"],
-          "gatewayIps": ["192.168.10.254", "1000::100:1"]
+          "gatewayIps": ["192.168.10.254", "1000::100:1"],
+          "relayAgentIps": ["10.0.0.1", "1000:100::100:1"]
         }
       ],
       "indirect": [
         {
           "dhcpServerConnectPoint": "of:0000000000000002/3",
-          "serverIps": ["172.168.10.3"]
+          "serverIps": ["172.168.10.3"],
+          "relayAgentIps": ["10.0.1.1"]
         }
       ]
     }