From 5f5ceb66a4c8b51f9647ea7acd8acd07a7843b1e Mon Sep 17 00:00:00 2001 From: Jordan Halterman Date: Mon, 1 Oct 2018 23:17:57 -0700 Subject: [PATCH] Normalize route objects stored in RouteTable ConsistentMultimap to avoid inconsistent serialization for different IpPrefix/IpAddress types Change-Id: I3e792fb92afdf388d2eaec5bd04dc47347f910f5 (cherry picked from commit 30ffafd7883b752fbc579582f6de5dc2e3c829d1) --- .../routeservice/store/DefaultRouteTable.java | 74 ++++++++++++++----- 1 file changed, 56 insertions(+), 18 deletions(-) diff --git a/apps/route-service/app/src/main/java/org/onosproject/routeservice/store/DefaultRouteTable.java b/apps/route-service/app/src/main/java/org/onosproject/routeservice/store/DefaultRouteTable.java index 476f3c0dd6..b52618ca3a 100644 --- a/apps/route-service/app/src/main/java/org/onosproject/routeservice/store/DefaultRouteTable.java +++ b/apps/route-service/app/src/main/java/org/onosproject/routeservice/store/DefaultRouteTable.java @@ -23,10 +23,10 @@ import java.util.concurrent.ExecutorService; import java.util.function.Consumer; import java.util.stream.Collectors; -import com.google.common.collect.ImmutableSet; import com.google.common.collect.Sets; import org.onlab.packet.IpAddress; import org.onlab.packet.IpPrefix; +import org.onosproject.cluster.NodeId; import org.onlab.util.KryoNamespace; import org.onosproject.routeservice.InternalRouteEvent; import org.onosproject.routeservice.Route; @@ -50,7 +50,12 @@ import static com.google.common.base.Preconditions.checkNotNull; public class DefaultRouteTable implements RouteTable { private final RouteTableId id; - private final ConsistentMultimap routes; + + // The route map stores RawRoute instead of Route to translate the polymorphic IpPrefix and IpAddress types + // into monomorphic types (specifically String). Using strings in the stored RawRoute is necessary to ensure + // the serialized bytes are consistent whether e.g. IpAddress or Ip4Address is used when storing a route. + private final ConsistentMultimap routes; + private final RouteStoreDelegate delegate; private final ExecutorService executor; private final RouteTableListener listener = new RouteTableListener(); @@ -89,13 +94,14 @@ public class DefaultRouteTable implements RouteTable { new InternalRouteEvent(InternalRouteEvent.Type.ROUTE_ADDED, routeSet))); } - private ConsistentMultimap buildRouteMap(StorageService storageService) { + private ConsistentMultimap buildRouteMap(StorageService storageService) { KryoNamespace routeTableSerializer = KryoNamespace.newBuilder() .register(KryoNamespaces.API) .register(Route.class) .register(Route.Source.class) + .register(RawRoute.class) .build(); - return storageService.consistentMultimapBuilder() + return storageService.consistentMultimapBuilder() .withName("onos-routes-" + id.name()) .withRelaxedReadConsistency() .withSerializer(Serializer.using(routeTableSerializer)) @@ -121,36 +127,37 @@ public class DefaultRouteTable implements RouteTable { @Override public void update(Route route) { - routes.put(route.prefix(), route); + routes.put(route.prefix().toString(), new RawRoute(route)); } @Override public void remove(Route route) { - routes.remove(route.prefix(), route); + routes.remove(route.prefix().toString(), new RawRoute(route)); } @Override public void replace(Route route) { - routes.replaceValues(route.prefix(), Sets.newHashSet(route)); + routes.replaceValues(route.prefix().toString(), Sets.newHashSet(new RawRoute(route))); } @Override public Collection getRoutes() { return routes.stream() .map(Map.Entry::getValue) - .collect(Collectors.groupingBy(Route::prefix)) + .collect(Collectors.groupingBy(RawRoute::prefix)) .entrySet() .stream() - .map(entry -> new RouteSet(id, entry.getKey(), ImmutableSet.copyOf(entry.getValue()))) + .map(entry -> new RouteSet(id, + IpPrefix.valueOf(entry.getKey()), + entry.getValue().stream().map(RawRoute::route).collect(Collectors.toSet()))) .collect(Collectors.toList()); } @Override public RouteSet getRoutes(IpPrefix prefix) { - Versioned> routeSet = routes.get(prefix); - + Versioned> routeSet = routes.get(prefix.toString()); if (routeSet != null) { - return new RouteSet(id, prefix, ImmutableSet.copyOf(routeSet.value())); + return new RouteSet(id, prefix, routeSet.value().stream().map(RawRoute::route).collect(Collectors.toSet())); } return null; } @@ -159,22 +166,25 @@ public class DefaultRouteTable implements RouteTable { public Collection getRoutesForNextHop(IpAddress nextHop) { return routes.stream() .map(Map.Entry::getValue) - .filter(r -> r.nextHop().equals(nextHop)) + .filter(r -> IpAddress.valueOf(r.nextHop()).equals(nextHop)) + .map(RawRoute::route) .collect(Collectors.toSet()); } private class RouteTableListener - implements MultimapEventListener { + implements MultimapEventListener { private InternalRouteEvent createRouteEvent( - InternalRouteEvent.Type type, MultimapEvent event) { - Collection currentRoutes = Versioned.valueOrNull(routes.get(event.key())); + InternalRouteEvent.Type type, MultimapEvent event) { + Collection currentRoutes = Versioned.valueOrNull(routes.get(event.key())); return new InternalRouteEvent(type, new RouteSet( - id, event.key(), currentRoutes != null ? ImmutableSet.copyOf(currentRoutes) : Collections.emptySet())); + id, IpPrefix.valueOf(event.key()), currentRoutes != null ? + currentRoutes.stream().map(RawRoute::route).collect(Collectors.toSet()) + : Collections.emptySet())); } @Override - public void event(MultimapEvent event) { + public void event(MultimapEvent event) { InternalRouteEvent ire = null; switch (event.type()) { case INSERT: @@ -190,4 +200,32 @@ public class DefaultRouteTable implements RouteTable { } } + /** + * Represents a route object stored in the underlying ConsistentMultimap. + */ + private static class RawRoute { + private Route.Source source; + private String prefix; + private String nextHop; + private NodeId sourceNode; + + RawRoute(Route route) { + this.source = route.source(); + this.prefix = route.prefix().toString(); + this.nextHop = route.nextHop().toString(); + this.sourceNode = route.sourceNode(); + } + + String prefix() { + return prefix; + } + + String nextHop() { + return nextHop; + } + + Route route() { + return new Route(source, IpPrefix.valueOf(prefix), IpAddress.valueOf(nextHop), sourceNode); + } + } }