Tweak subnets so that subnets attached to parent forwarding contexts

will properly reply to the ARP requests for the virtual router IP
address.

Change-Id: Ibc639d2bfe0e397aa7e746adf99d61f30dd4dc78
Signed-off-by: Rob Adams <readams@readams.net>

diff --git a/groupbasedpolicy/src/main/java/org/opendaylight/groupbasedpolicy/renderer/ofoverlay/flow/DestinationMapper.java b/groupbasedpolicy/src/main/java/org/opendaylight/groupbasedpolicy/renderer/ofoverlay/flow/DestinationMapper.java
index 6774eb8d04e26a9655ac4ffe0e702b4b93675080..e1e527276479abf76266debfcd226ddb33279cae 100644 (file)
--- a/groupbasedpolicy/src/main/java/org/opendaylight/groupbasedpolicy/renderer/ofoverlay/flow/DestinationMapper.java
+++ b/groupbasedpolicy/src/main/java/org/opendaylight/groupbasedpolicy/renderer/ofoverlay/flow/DestinationMapper.java
@@ -19,7 +19,6 @@ import java.util.Objects;
 import java.util.Set;
 
 import org.opendaylight.controller.md.sal.binding.api.ReadWriteTransaction;
-import org.opendaylight.controller.sal.utils.HexEncode;
 import org.opendaylight.groupbasedpolicy.renderer.ofoverlay.PolicyManager.Dirty;
 import org.opendaylight.groupbasedpolicy.renderer.ofoverlay.flow.FlowUtils.RegMatch;
 import org.opendaylight.groupbasedpolicy.resolver.ConditionGroup;
@@ -135,7 +134,7 @@ public class DestinationMapper extends FlowTable {
         IndexedTenant tenant = ctx.policyResolver.getTenant(key.getTenantId());
         EndpointGroup eg = tenant.getEndpointGroup(key.getEgId());
         L2FloodDomain fd = tenant.resolveL2FloodDomain(eg.getNetworkDomain());
-        Subnet sn = tenant.resolveSubnet(eg.getNetworkDomain());
+        Collection<Subnet> sns = tenant.resolveSubnets(eg.getNetworkDomain());
         L3Context l3c = tenant.resolveL3Context(eg.getNetworkDomain());
         int l3Id = 0;
 
@@ -182,11 +181,21 @@ public class DestinationMapper extends FlowTable {
                                                              groupAction(Long.valueOf(fdId)))));
             writeFlow(t, tiid, flow.build());
         }
-        
+        for (Subnet sn : sns) {
+            writeRouterArpFlow(t, tiid, flowMap, nodeId, sn, l3Id);
+        }
+    }
+
+    private void writeRouterArpFlow(ReadWriteTransaction t,
+                                    InstanceIdentifier<Table> tiid,
+                                    Map<String, FlowCtx> flowMap, 
+                                    NodeId nodeId,
+                                    Subnet sn,
+                                    int l3Id) {
         if (sn != null && sn.getVirtualRouterIp() != null) {
             if (sn.getVirtualRouterIp().getIpv4Address() != null) {
                 String ikey = sn.getVirtualRouterIp().getIpv4Address().getValue();
-                flowId = new FlowId(new StringBuffer()
+                FlowId flowId = new FlowId(new StringBuffer()
                     .append("routerarp|")
                     .append(sn.getId().getValue())
                     .append("|")
@@ -204,10 +213,8 @@ public class DestinationMapper extends FlowTable {
                     addNxRegMatch(mb, RegMatch.of(NxmNxReg6.class,
                                                   Long.valueOf(l3Id)));
                     BigInteger routerMac = 
-                            new BigInteger(1, HexEncode
-                                           .bytesFromHexString(ROUTER_MAC
-                                                               .getValue()));
-                    /* XXX - TODO add output to inport action */
+                            new BigInteger(1, bytesFromHexString(ROUTER_MAC
+                                                                 .getValue()));
                     FlowBuilder flowb = base()
                          .setPriority(150)
                          .setId(flowId)
@@ -228,7 +235,7 @@ public class DestinationMapper extends FlowTable {
             }
         }
     }
-
+    
     private void syncEP(ReadWriteTransaction t,
                         InstanceIdentifier<Table> tiid,
                         Map<String, FlowCtx> flowMap, 
@@ -445,4 +452,18 @@ public class DestinationMapper extends FlowTable {
             }
         }
     }
+    
+    static byte[] bytesFromHexString(String values) {
+        String target = "";
+        if (values != null) {
+            target = values;
+        }
+        String[] octets = target.split(":");
+
+        byte[] ret = new byte[octets.length];
+        for (int i = 0; i < octets.length; i++) {
+            ret[i] = Integer.valueOf(octets[i], 16).byteValue();
+        }
+        return ret;
+    }
 }

diff --git a/groupbasedpolicy/src/main/java/org/opendaylight/groupbasedpolicy/resolver/IndexedTenant.java b/groupbasedpolicy/src/main/java/org/opendaylight/groupbasedpolicy/resolver/IndexedTenant.java
index ffe055cd3e356eb4f1b2f5123a56aa33a699d364..ec67c921dc33c16a1a80257b8f94a3c99ada732b 100644 (file)
--- a/groupbasedpolicy/src/main/java/org/opendaylight/groupbasedpolicy/resolver/IndexedTenant.java
+++ b/groupbasedpolicy/src/main/java/org/opendaylight/groupbasedpolicy/resolver/IndexedTenant.java
@@ -8,9 +8,11 @@
 
 package org.opendaylight.groupbasedpolicy.resolver;
 
+import java.util.Collection;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Map;
+import java.util.Set;
 
 import javax.annotation.concurrent.Immutable;
 
@@ -19,6 +21,7 @@ import org.opendaylight.yang.gen.v1.urn.opendaylight.groupbasedpolicy.common.rev
 import org.opendaylight.yang.gen.v1.urn.opendaylight.groupbasedpolicy.common.rev140421.ContractId;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.groupbasedpolicy.common.rev140421.EndpointGroupId;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.groupbasedpolicy.common.rev140421.NetworkDomainId;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.groupbasedpolicy.common.rev140421.SubnetId;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.groupbasedpolicy.policy.rev140421.NetworkDomain;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.groupbasedpolicy.policy.rev140421.tenants.Tenant;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.groupbasedpolicy.policy.rev140421.tenants.tenant.Contract;
@@ -31,6 +34,9 @@ import org.opendaylight.yang.gen.v1.urn.opendaylight.groupbasedpolicy.policy.rev
 import org.opendaylight.yang.gen.v1.urn.opendaylight.groupbasedpolicy.policy.rev140421.tenants.tenant.subject.feature.instances.ActionInstance;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.groupbasedpolicy.policy.rev140421.tenants.tenant.subject.feature.instances.ClassifierInstance;
 
+import com.google.common.base.Function;
+import com.google.common.collect.Collections2;
+
 /**
  * Wrap some convenient indexes around a {@link Tenant} object
  * @author readams
@@ -50,6 +56,7 @@ public class IndexedTenant {
             new HashMap<>();
     private final Map<ActionName, ActionInstance> actions =
             new HashMap<>();
+    private final Map<String, Set<SubnetId>> subnetMap = new HashMap<>();
     
     public IndexedTenant(Tenant tenant) {
         super();
@@ -84,6 +91,12 @@ public class IndexedTenant {
         if (tenant.getSubnet() != null) {
             for (Subnet s : tenant.getSubnet()) {
                 networkDomains.put(s.getId().getValue(), s);
+                Set<SubnetId> sset = subnetMap.get(s.getParent().getValue());
+                if (sset == null) {
+                    subnetMap.put(s.getParent().getValue(), 
+                                  sset = new HashSet<SubnetId>());
+                }
+                sset.add(s.getId());
             }
         }
         if (tenant.getSubjectFeatureInstances() != null) {
@@ -192,15 +205,38 @@ public class IndexedTenant {
     }
 
     /**
-     * If the specified network domain represents a subnet, return it.
+     * Resolve all subnets applicable to the given network domain ID
      * @param id the {@link NetworkDomainId}
-     * @return the {@link Subnet} if it exists, or <code>null</code> otherwise
+     * @return the set of subnets.  Cannot be null, but could be empty.
      */
-    public Subnet resolveSubnet(NetworkDomainId id) {
-        NetworkDomain d = networkDomains.get(id.getValue());
-        if (d == null) return null;
-        if (d instanceof Subnet) return (Subnet)d;
-        return null;
+    public Collection<Subnet> resolveSubnets(NetworkDomainId id) {
+        Set<SubnetId> sset = new HashSet<>();
+        HashSet<NetworkDomainId> visited = new HashSet<>();        
+        while (id != null) {
+            if (visited.contains(id)) return null;
+            visited.add(id);
+            Set<SubnetId> cursset = subnetMap.get(id.getValue());
+            if (cursset != null)
+                sset.addAll(cursset);
+            NetworkDomain d = networkDomains.get(id.getValue());
+            if (d == null) break;
+            if (d instanceof Subnet) {
+                id = ((Subnet)d).getParent();
+                sset.add(((Subnet) d).getId());
+            } 
+            else if (d instanceof L2BridgeDomain)
+                id = ((L2BridgeDomain)d).getParent();
+            else if (d instanceof L2FloodDomain)
+                id = ((L2FloodDomain)d).getParent();
+            else
+                id = null;
+        }
+        return Collections2.transform(sset, new Function<SubnetId, Subnet>() {
+            @Override
+            public Subnet apply(SubnetId input) {
+                return (Subnet)networkDomains.get(input.getValue());
+            }
+        });
     }
 
     // ******
@@ -244,10 +280,12 @@ public class IndexedTenant {
             if (domainClass.isInstance(d)) return domainClass.cast(d);
             if (d instanceof Subnet)
                 id = ((Subnet)d).getParent();
-            if (d instanceof L2BridgeDomain)
+            else if (d instanceof L2BridgeDomain)
                 id = ((L2BridgeDomain)d).getParent();
-            if (d instanceof L2FloodDomain)
+            else if (d instanceof L2FloodDomain)
                 id = ((L2FloodDomain)d).getParent();
+            else
+                id = null;
         }
         return null;
     }

diff --git a/groupbasedpolicy/src/test/java/org/opendaylight/groupbasedpolicy/renderer/ofoverlay/flow/DestinationMapperTest.java b/groupbasedpolicy/src/test/java/org/opendaylight/groupbasedpolicy/renderer/ofoverlay/flow/DestinationMapperTest.java
index d5f8d00ade004e535a07c6fe8bd440ced72f9213..12326eed9b2c93f7048facf3ff6897bb8ff27834 100644 (file)
--- a/groupbasedpolicy/src/test/java/org/opendaylight/groupbasedpolicy/renderer/ofoverlay/flow/DestinationMapperTest.java
+++ b/groupbasedpolicy/src/test/java/org/opendaylight/groupbasedpolicy/renderer/ofoverlay/flow/DestinationMapperTest.java
@@ -20,7 +20,6 @@ import org.mockito.ArgumentCaptor;
 import org.mockito.Matchers;
 import org.opendaylight.controller.md.sal.binding.api.ReadWriteTransaction;
 import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType;
-import org.opendaylight.controller.sal.utils.HexEncode;
 import org.opendaylight.groupbasedpolicy.renderer.ofoverlay.flow.FlowTable.FlowCtx;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.inet.types.rev100924.IpAddress;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.inet.types.rev100924.Ipv4Address;
@@ -112,7 +111,7 @@ public class DestinationMapperTest extends FlowTableTest {
                 assertEquals(nxMoveArpShaToArpThaAction(),
                              actions.get(3).getAction());
                 assertEquals(Integer.valueOf(3), actions.get(3).getOrder());
-                assertEquals(nxLoadArpShaAction(new BigInteger(1, HexEncode
+                assertEquals(nxLoadArpShaAction(new BigInteger(1, DestinationMapper
                                                                .bytesFromHexString(DestinationMapper.ROUTER_MAC
                                                                                    .getValue()))),
                              actions.get(4).getAction());
@@ -121,7 +120,8 @@ public class DestinationMapperTest extends FlowTableTest {
                              actions.get(5).getAction());
                 assertEquals(Integer.valueOf(5), actions.get(5).getOrder());
                 assertTrue(nxLoadArpSpaAction("10.0.0.1").equals(actions.get(6).getAction()) ||
-                           nxLoadArpSpaAction("10.0.0.2").equals(actions.get(6).getAction()));
+                           nxLoadArpSpaAction("10.0.1.1").equals(actions.get(6).getAction()) ||
+                           nxLoadArpSpaAction("10.0.2.1").equals(actions.get(6).getAction()));
                 assertEquals(Integer.valueOf(6), actions.get(6).getOrder());
                 count += 1;
             } else if (Objects.equals(localEp.getMacAddress(),
@@ -228,7 +228,7 @@ public class DestinationMapperTest extends FlowTableTest {
                 count += 1;
             }
         }
-        assertEquals(7, count);
+        assertEquals(9, count);
 
         t = dosync(flowMap);
         verify(t, never()).put(any(LogicalDatastoreType.class), 

diff --git a/groupbasedpolicy/src/test/java/org/opendaylight/groupbasedpolicy/renderer/ofoverlay/flow/OfTableTest.java b/groupbasedpolicy/src/test/java/org/opendaylight/groupbasedpolicy/renderer/ofoverlay/flow/OfTableTest.java
index 0c62fd0f89f97f5911b14b03230845e03e0c8b4a..aa441aef7623d10f8bb23872cb9574dc9b874243 100644 (file)
--- a/groupbasedpolicy/src/test/java/org/opendaylight/groupbasedpolicy/renderer/ofoverlay/flow/OfTableTest.java
+++ b/groupbasedpolicy/src/test/java/org/opendaylight/groupbasedpolicy/renderer/ofoverlay/flow/OfTableTest.java
@@ -81,6 +81,7 @@ public class OfTableTest {
     L2FloodDomainId fd = new L2FloodDomainId("98e1439e-52d2-46f8-bd69-5136e6088771");
     SubnetId sub = new SubnetId("4fcf8dfc-53b5-4aef-84d3-6b5586992fcb");
     SubnetId sub2 = new SubnetId("c285a59f-fcb8-42e6-bf29-87ea522fd626");
+    SubnetId sub3 = new SubnetId("a0380d52-2a25-48ef-882c-a4d4cd9e00ec");
     TenantId tid = new TenantId("1118c691-8520-47ad-80b8-4cf5e3fe3302");
     EndpointGroupId eg = new EndpointGroupId("36dec84a-08c7-497b-80b6-a0035af72a12");
     EndpointGroupId eg2 = new EndpointGroupId("632e5e11-7988-4eb5-8fe6-6c182d890276");
@@ -146,6 +147,12 @@ public class OfTableTest {
                     .setParent(fd)
                     .setIpPrefix(new IpPrefix(new Ipv4Prefix("10.0.0.1/24")))
                     .setVirtualRouterIp(new IpAddress(new Ipv4Address("10.0.0.1")))
+                    .build(),
+                new SubnetBuilder()
+                    .setId(sub3)
+                    .setParent(bd)
+                    .setIpPrefix(new IpPrefix(new Ipv4Prefix("10.0.2.1/24")))
+                    .setVirtualRouterIp(new IpAddress(new Ipv4Address("10.0.2.1")))
                     .build()))
             .setSubjectFeatureInstances(new SubjectFeatureInstancesBuilder()
                 .setClassifierInstance(ImmutableList.of(new ClassifierInstanceBuilder()

diff --git a/groupbasedpolicy/src/test/java/org/opendaylight/groupbasedpolicy/resolver/IndexedTenantTest.java b/groupbasedpolicy/src/test/java/org/opendaylight/groupbasedpolicy/resolver/IndexedTenantTest.java
index 7582f5ce24245609fc52afe7502810b1570bdbd6..1ffc6f91b0bc0e565f4b569203fa1a5d8dfeadc2 100644 (file)
--- a/groupbasedpolicy/src/test/java/org/opendaylight/groupbasedpolicy/resolver/IndexedTenantTest.java
+++ b/groupbasedpolicy/src/test/java/org/opendaylight/groupbasedpolicy/resolver/IndexedTenantTest.java
@@ -8,6 +8,8 @@
 
 package org.opendaylight.groupbasedpolicy.resolver;
 
+import java.util.Collection;
+
 import org.junit.Test;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.groupbasedpolicy.common.rev140421.L2BridgeDomainId;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.groupbasedpolicy.common.rev140421.L2FloodDomainId;
@@ -33,6 +35,7 @@ public class IndexedTenantTest {
     @Test
     public void testResolveND() throws Exception {
         SubnetId sid = new SubnetId("dd25397d-d829-4c8d-8c01-31f129b8de8f");
+        SubnetId sid2 = new SubnetId("c752ba40-40aa-4a47-8138-9b7175b854fa");
         L3ContextId l3id = new L3ContextId("f2311f52-890f-4095-8b85-485ec8b92b3c");
         L2BridgeDomainId bdid= new L2BridgeDomainId("70aeb9ea-4ca1-4fb9-9780-22b04b84a0d6");
         L2FloodDomainId fdid = new L2FloodDomainId("252fbac6-bb6e-4d16-808d-6f56d20e5cca");
@@ -47,8 +50,11 @@ public class IndexedTenantTest {
         Subnet s = new SubnetBuilder()
             .setParent(fdid)
             .setId(sid).build();
+        Subnet s2 = new SubnetBuilder()
+            .setParent(bdid)
+            .setId(sid2).build();
         Tenant t = new TenantBuilder()
-            .setSubnet(ImmutableList.of(s))
+            .setSubnet(ImmutableList.of(s, s2))
             .setL2BridgeDomain(ImmutableList.of(bd))
             .setL3Context(ImmutableList.of(l3c))
             .setL2FloodDomain(ImmutableList.of(fd))
@@ -56,7 +62,9 @@ public class IndexedTenantTest {
         IndexedTenant it = new IndexedTenant(t);
 
         assertNotNull(it.getNetworkDomain(sid));
-        assertEquals(sid, it.resolveSubnet(sid).getId());
+        Collection<Subnet> sns = it.resolveSubnets(sid);
+        assertTrue(sns.contains(s));
+        assertTrue(sns.contains(s2));
         assertEquals(l3id, it.resolveL3Context(sid).getId());
         assertEquals(bdid, it.resolveL2BridgeDomain(sid).getId());
         assertEquals(fdid, it.resolveL2FloodDomain(sid).getId());