merge

@@ -74,13 +74,12 @@ public class ReactiveForwarding {
 
         @Override
         public void process(PacketContext context) {
-            /*
-             *  stop processing if the packet has been handled,
-             *  we can't do any more to it
-             */
+            // Stop processing if the packet has been handled, since we
+            // can't do any more to it.
             if (context.isHandled()) {
                 return;
             }
+
             InboundPacket pkt = context.inPacket();
             HostId id = HostId.hostId(pkt.parsed().getDestinationMAC());
 
@@ -100,7 +99,6 @@ public class ReactiveForwarding {
 
             // Otherwise, get a set of paths that lead from here to the
             // destination edge switch.
-
             Set<Path> paths = topologyService.getPaths(topologyService.currentTopology(),
                     context.inPacket().receivedFrom().deviceId(),
                     dst.location().deviceId());
@@ -137,9 +135,8 @@ public class ReactiveForwarding {
 
     // Floods the specified packet.
     private void flood(PacketContext context) {
-        boolean canBcast = topologyService.isBroadcastPoint(topologyService.currentTopology(),
-                context.inPacket().receivedFrom());
-        if (canBcast) {
+        if (topologyService.isBroadcastPoint(topologyService.currentTopology(),
+                context.inPacket().receivedFrom())) {
             packetOutFlood(context);
         } else {
             context.block();
@@ -173,6 +170,10 @@ public class ReactiveForwarding {
 
         flowRuleService.applyFlowRules(f);
 
+        // we don't yet support bufferids in the flowservice so packet out and
+        // then install a flowmod.
+        context.treatmentBuilder().add(Instructions.createOutput(portNumber));
+        context.send();
     }
 
 }

 package org.onlab.onos.net;
 
-import java.util.Objects;
-
 /**
  * Representation of a network edge location where an end-station host is
  * connected.
  */
 public class HostLocation extends ConnectPoint {
 
+    // Note that time is explicitly excluded from the notion of equality.
     private final long time;
 
     public HostLocation(DeviceId deviceId, PortNumber portNumber, long time) {
@@ -25,18 +24,4 @@ public class HostLocation extends ConnectPoint {
         return time;
     }
 
-    @Override
-    public int hashCode() {
-        return 31 * super.hashCode() + Objects.hash(time);
-    }
-
-    @Override
-    public boolean equals(Object obj) {
-        if (obj instanceof HostLocation) {
-            final HostLocation other = (HostLocation) obj;
-            return super.equals(obj) && Objects.equals(this.time, other.time);
-        }
-        return false;
-    }
-
 }

@@ -43,7 +43,6 @@ public interface HostService {
      * @param vlanId vlan identifier
      * @return set of hosts in the given vlan id
      */
-    // FIXME: change long to VLanId
     Set<Host> getHostsByVlan(VlanId vlanId);
 
     /**
@@ -62,6 +61,8 @@ public interface HostService {
      */
     Set<Host> getHostsByIp(IpAddress ip);
 
+    // TODO: consider adding Host getHostByIp(IpAddress ip, VlanId vlan);
+
     /**
      * Returns the set of hosts whose most recent location is the specified
      * connection point.

 /**
- * Core subsystem for tracking infrastructure devices.
+ * Core subsystem for tracking global inventory of infrastructure devices.
  */
 package org.onlab.onos.net.trivial.device.impl;

+/**
+ * Core subsystem for tracking and manipulating global flow state.
+ */
+package org.onlab.onos.net.trivial.flow.impl;

@@ -48,7 +48,7 @@ public class SimpleHostStore {
      * @return appropriate event or null if no change resulted
      */
     HostEvent createOrUpdateHost(ProviderId providerId, HostId hostId,
-            HostDescription hostDescription) {
+                                 HostDescription hostDescription) {
         Host host = hosts.get(hostId);
         if (host == null) {
             return createHost(providerId, hostId, hostDescription);
@@ -58,12 +58,12 @@ public class SimpleHostStore {
 
     // creates a new host and sends HOST_ADDED
     private HostEvent createHost(ProviderId providerId, HostId hostId,
-            HostDescription descr) {
+                                 HostDescription descr) {
         DefaultHost newhost = new DefaultHost(providerId, hostId,
-                descr.hwAddress(),
-                descr.vlan(),
-                descr.location(),
-                descr.ipAddresses());
+                                              descr.hwAddress(),
+                                              descr.vlan(),
+                                              descr.location(),
+                                              descr.ipAddresses());
         synchronized (this) {
             hosts.put(hostId, newhost);
             locations.put(descr.location(), newhost);
@@ -73,23 +73,23 @@ public class SimpleHostStore {
 
     // checks for type of update to host, sends appropriate event
     private HostEvent updateHost(ProviderId providerId, Host host,
-            HostDescription descr) {
+                                 HostDescription descr) {
         DefaultHost updated;
         HostEvent event;
-        // Consider only actual location (not timestamp) change?
-        if (!(host.location().port().equals(descr.location().port()))) {
+        if (!host.location().equals(descr.location())) {
             updated = new DefaultHost(providerId, host.id(),
-                    host.mac(),
-                    host.vlan(),
-                    descr.location(),
-                    host.ipAddresses());
+                                      host.mac(),
+                                      host.vlan(),
+                                      descr.location(),
+                                      host.ipAddresses());
             event = new HostEvent(HOST_MOVED, updated);
+
         } else if (!(host.ipAddresses().equals(descr.ipAddresses()))) {
             updated = new DefaultHost(providerId, host.id(),
-                    host.mac(),
-                    host.vlan(),
-                    descr.location(),
-                    descr.ipAddresses());
+                                      host.mac(),
+                                      host.vlan(),
+                                      descr.location(),
+                                      descr.ipAddresses());
             event = new HostEvent(HOST_UPDATED, updated);
         } else {
             return null;
@@ -134,7 +134,7 @@ public class SimpleHostStore {
      * @return iterable collection of all hosts
      */
     Iterable<Host> getHosts() {
-        return Collections.unmodifiableSet(new HashSet<Host>(hosts.values()));
+        return Collections.unmodifiableSet(new HashSet<>(hosts.values()));
     }
 
     /**
@@ -154,7 +154,7 @@ public class SimpleHostStore {
      * @return set of hosts in the vlan
      */
     Set<Host> getHosts(VlanId vlanId) {
-        Set<Host> vlanset = new HashSet<Host>();
+        Set<Host> vlanset = new HashSet<>();
         for (Host h : hosts.values()) {
             if (h.vlan().equals(vlanId)) {
                 vlanset.add(h);

 /**
- * Core subsystem for tracking edn-station hosts.
+ * Core subsystem for tracking global inventory of end-station hosts.
  */
 package org.onlab.onos.net.trivial.host.impl;

 /**
- * Core subsystem for tracking infrastructure links.
+ * Core subsystem for tracking global inventory of infrastructure links.
  */
 package org.onlab.onos.net.trivial.link.impl;

 /**
  * Core subsystem for processing inbound packets and emitting outbound packets.
+ * Processing of inbound packets is always in the local context only, but
+ * emitting outbound packets allows for cluster-wide operation.
  */
 package org.onlab.onos.net.trivial.packet.impl;

@@ -142,6 +142,15 @@ public class DefaultTopology extends AbstractModel implements Topology {
         return clusters.get(clusterId);
     }
 
+    /**
+     * Returns the topology cluster that contains the given device.
+     *
+     * @param deviceId device identifier
+     * @return topology cluster
+     */
+    TopologyCluster getCluster(DeviceId deviceId) {
+        return clustersByDevice.get(deviceId);
+    }
 
     /**
      * Returns the set of cluster devices.
@@ -174,13 +183,13 @@ public class DefaultTopology extends AbstractModel implements Topology {
     }
 
     /**
-     * Indicates whether the given point is part of a broadcast tree.
+     * Indicates whether the given point is part of a broadcast set.
      *
      * @param connectPoint connection point
-     * @return true if in broadcast tree
+     * @return true if in broadcast set
      */
-    boolean isInBroadcastTree(ConnectPoint connectPoint) {
-        // Any non-infrastructure, i.e. edge points are assumed to be OK
+    boolean isBroadcastPoint(ConnectPoint connectPoint) {
+        // Any non-infrastructure, i.e. edge points are assumed to be OK.
         if (!isInfrastructure(connectPoint)) {
             return true;
         }
@@ -191,13 +200,23 @@ public class DefaultTopology extends AbstractModel implements Topology {
             throw new IllegalArgumentException("No cluster found for device " + connectPoint.deviceId());
         }
 
-        // If the broadcast tree is null or empty, or if the point explicitly
-        // belongs to the broadcast tree points, return true;
+        // If the broadcast set is null or empty, or if the point explicitly
+        // belongs to it, return true;
         Set<ConnectPoint> points = broadcastSets.get(cluster.id());
         return points == null || points.isEmpty() || points.contains(connectPoint);
     }
 
     /**
+     * Returns the size of the cluster broadcast set.
+     *
+     * @param clusterId cluster identifier
+     * @return size of the cluster broadcast set
+     */
+    int broadcastSetSize(ClusterId clusterId) {
+        return broadcastSets.get(clusterId).size();
+    }
+
+    /**
      * Returns the set of pre-computed shortest paths between source and
      * destination devices.
      *

@@ -144,7 +144,7 @@ class SimpleTopologyStore {
      * @return true if broadcast allowed; false otherwise
      */
     boolean isBroadcastPoint(DefaultTopology topology, ConnectPoint connectPoint) {
-        return topology.isInBroadcastTree(connectPoint);
+        return topology.isBroadcastPoint(connectPoint);
     }
 
     /**

 /**
- * Core subsystem for tracking consistent topology graph views.
+ * Core subsystem for tracking global & consistent topology graph views.
  */
 package org.onlab.onos.net.trivial.topology.impl;

+package org.onlab.onos.net.trivial.topology.impl;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.onlab.onos.net.ConnectPoint;
+import org.onlab.onos.net.Device;
+import org.onlab.onos.net.DeviceId;
+import org.onlab.onos.net.Link;
+import org.onlab.onos.net.Path;
+import org.onlab.onos.net.PortNumber;
+import org.onlab.onos.net.provider.ProviderId;
+import org.onlab.onos.net.topology.ClusterId;
+import org.onlab.onos.net.topology.GraphDescription;
+import org.onlab.onos.net.topology.LinkWeight;
+import org.onlab.onos.net.topology.TopologyCluster;
+import org.onlab.onos.net.topology.TopologyEdge;
+
+import java.util.Set;
+
+import static com.google.common.collect.ImmutableSet.of;
+import static org.junit.Assert.*;
+import static org.onlab.onos.net.DeviceId.deviceId;
+import static org.onlab.onos.net.PortNumber.portNumber;
+import static org.onlab.onos.net.trivial.topology.impl.SimpleTopologyManagerTest.device;
+import static org.onlab.onos.net.trivial.topology.impl.SimpleTopologyManagerTest.link;
+
+/**
+ * Test of the default topology implementation.
+ */
+public class DefaultTopologyTest {
+
+    public static final ProviderId PID = new ProviderId("foo.bar");
+
+    public static final DeviceId D1 = deviceId("of:1");
+    public static final DeviceId D2 = deviceId("of:2");
+    public static final DeviceId D3 = deviceId("of:3");
+    public static final DeviceId D4 = deviceId("of:4");
+    public static final DeviceId D5 = deviceId("of:5");
+
+    public static final PortNumber P1 = portNumber(1);
+    public static final PortNumber P2 = portNumber(2);
+
+    public static final LinkWeight WEIGHT = new LinkWeight() {
+        @Override
+        public double weight(TopologyEdge edge) {
+            return edge.src().deviceId().equals(D4) ||
+                    edge.dst().deviceId().equals(D4) ? 2.0 : 1.0;
+        }
+    };
+
+    private DefaultTopology dt;
+
+    @Before
+    public void setUp() {
+        long now = System.currentTimeMillis();
+        Set<Device> devices = of(device("1"), device("2"),
+                                 device("3"), device("4"),
+                                 device("5"));
+        Set<Link> links = of(link("1", 1, "2", 1), link("2", 1, "1", 1),
+                             link("3", 2, "2", 2), link("2", 2, "3", 2),
+                             link("1", 3, "4", 3), link("4", 3, "1", 3),
+                             link("3", 4, "4", 4), link("4", 4, "3", 4));
+        GraphDescription graphDescription =
+                new DefaultGraphDescription(now, devices, links);
+
+        dt = new DefaultTopology(PID, graphDescription);
+        assertEquals("incorrect supplier", PID, dt.providerId());
+        assertEquals("incorrect time", now, dt.time());
+        assertEquals("incorrect device count", 5, dt.deviceCount());
+        assertEquals("incorrect link count", 8, dt.linkCount());
+        assertEquals("incorrect cluster count", 2, dt.clusterCount());
+        assertEquals("incorrect broadcast set size", 6,
+                     dt.broadcastSetSize(ClusterId.clusterId(0)));
+    }
+
+    @Test
+    public void pathRelated() {
+        Set<Path> paths = dt.getPaths(D1, D2);
+        assertEquals("incorrect path count", 1, paths.size());
+
+        paths = dt.getPaths(D1, D3);
+        assertEquals("incorrect path count", 2, paths.size());
+
+        paths = dt.getPaths(D1, D5);
+        assertTrue("no paths expected", paths.isEmpty());
+
+        paths = dt.getPaths(D1, D3, WEIGHT);
+        assertEquals("incorrect path count", 1, paths.size());
+    }
+
+    @Test
+    public void pointRelated() {
+        assertTrue("should be infrastructure point",
+                   dt.isInfrastructure(new ConnectPoint(D1, P1)));
+        assertFalse("should not be infrastructure point",
+                    dt.isInfrastructure(new ConnectPoint(D1, P2)));
+    }
+
+    @Test
+    public void clusterRelated() {
+        Set<TopologyCluster> clusters = dt.getClusters();
+        assertEquals("incorrect cluster count", 2, clusters.size());
+
+        TopologyCluster c = dt.getCluster(D1);
+        Set<DeviceId> devs = dt.getClusterDevices(c);
+        assertEquals("incorrect cluster device count", 4, devs.size());
+        assertTrue("cluster should contain D2", devs.contains(D2));
+        assertFalse("cluster should not contain D5", devs.contains(D5));
+    }
+
+}

@@ -24,8 +24,6 @@ import org.projectfloodlight.openflow.types.*;
 import org.projectfloodlight.openflow.util.*;
 import org.projectfloodlight.openflow.exceptions.*;
 import org.jboss.netty.buffer.ChannelBuffer;
-
-import java.nio.ByteBuffer;
 import java.util.Set;
 
 abstract class OFActionBsnVer13 {
@@ -38,7 +36,7 @@ abstract class OFActionBsnVer13 {
 
     static class Reader implements OFMessageReader<OFActionBsn> {
         @Override
-        public OFActionBsn readFrom(ByteBuffer bb) throws OFParseError {
+        public OFActionBsn readFrom(ChannelBuffer bb) throws OFParseError {
             if(bb.readableBytes() < MINIMUM_LENGTH)
                 return null;
             int start = bb.readerIndex();

@@ -101,7 +101,6 @@ public class OpenFlowRuleProvider extends AbstractProvider implements FlowRulePr
         for (int i = 0; i < flowRules.length; i++) {
             applyRule(flowRules[i]);
         }
-
     }
 
     private void applyRule(FlowRule flowRule) {
@@ -120,9 +119,7 @@ public class OpenFlowRuleProvider extends AbstractProvider implements FlowRulePr
                 .setHardTimeout(10)
                 .setPriority(flowRule.priority())
                 .build();
-
         sw.sendMsg(fm);
-
     }
 
     private List<OFAction> buildActions(List<Instruction> instructions, OFFactory factory) {

@@ -12,7 +12,7 @@ export KARAF_LOG=$KARAF/data/log/karaf.log
 
 # Setup a path
 export PS=":"
-export PATH="$PATH:$ONOS_ROOT/tools/dev;$ONOS_ROOT/tools/package"
+export PATH="$PATH:$ONOS_ROOT/tools/dev:$ONOS_ROOT/tools/package"
 export PATH="$PATH:$MAVEN/bin:$KARAF/bin"
 export PATH="$PATH:."
 

@@ -29,18 +29,13 @@ cd $ONOS_STAGE
 # Unroll the Apache Karaf bits and make the ONOS top-level directories.
 unzip $KARAF_ZIP
 mkdir bin
-mkdir lib
 
 # Stage the ONOS admin scripts
 cp -r $ONOS_ROOT/tools/package/bin .
 
 # Stage the ONOS bundles
-mkdir -p lib/org/onlab 
-cp -r $M2_REPO/org/onlab lib/org
-
-
-# Patch the Apache Karaf distribution file to point to the lib as maven repo
-#perl -pi.old -e "s|^org.ops4j.pax.url.mvn.repositories= |org.ops4j.pax.url.mvn.repositories= \\\n    file:../../lib, |" $ONOS_STAGE/$KARAF_DIST/etc/org.ops4j.pax.url.mvn.cfg
+mkdir -p system/org/onlab 
+cp -r $M2_REPO/org/onlab system/org/
 
 # Patch the Apache Karaf distribution file to add ONOS features repository
 perl -pi.old -e "s|^(featuresRepositories=.*)|\1,mvn:org.onlab.onos/onos-features/$ONOS_VERSION/xml/features|" \

@@ -5,7 +5,7 @@ import java.util.Arrays;
 /**
  * A class representing an IPv4 address.
  */
-public class IpAddress {
+public final class IpAddress {
 
     //IP Versions
     public enum Version { INET, INET6 };
@@ -14,13 +14,30 @@ public class IpAddress {
     public static final int INET_LEN = 4;
     public static final int INET6_LEN = 16;
 
+    //maximum CIDR value
+    public static final int MAX_INET_MASK = 32;
+    public static final int DEFAULT_MASK = 0;
+
+    /**
+     * Default value indicating an unspecified address.
+     */
+    public static final byte [] ANY = new byte [] {0, 0, 0, 0};
+
     protected Version version;
-    //does it make more sense to have a integral address?
+
     protected byte[] octets;
+    protected int netmask;
+
+    private IpAddress(Version ver, byte[] octets, int netmask) {
+        this.version = ver;
+        this.octets = Arrays.copyOf(octets, INET_LEN);
+        this.netmask = netmask;
+    }
 
-    protected IpAddress(Version ver, byte[] octets) {
+    private IpAddress(Version ver, byte[] octets) {
         this.version = ver;
         this.octets = Arrays.copyOf(octets, INET_LEN);
+        this.netmask = DEFAULT_MASK;
     }
 
     /**
@@ -34,38 +51,87 @@ public class IpAddress {
     }
 
     /**
-     * Converts an integer into an IPv4 address.
+     * Converts a byte array into an IP address.
      *
-     * @param address an integer representing an IP value
+     * @param address a byte array
+     * @param netmask the CIDR value subnet mask
      * @return an IP address
      */
-    public static IpAddress valueOf(int address) {
+    public static IpAddress valueOf(byte [] address, int netmask) {
+        return new IpAddress(Version.INET, address, netmask);
+    }
+
+    /**
+     * Helper to convert an integer into a byte array.
+     *
+     * @param address the integer to convert
+     * @return a byte array
+     */
+    private static byte [] bytes(int address) {
         byte [] bytes = new byte [INET_LEN];
         for (int i = 0; i < INET_LEN; i++) {
             bytes[i] = (byte) ((address >> (INET_LEN - (i + 1)) * 8) & 0xff);
         }
 
-        return new IpAddress(Version.INET, bytes);
+        return bytes;
+    }
+
+    /**
+     * Converts an integer into an IPv4 address.
+     *
+     * @param address an integer representing an IP value
+     * @return an IP address
+     */
+    public static IpAddress valueOf(int address) {
+        return new IpAddress(Version.INET, bytes(address));
+    }
+
+    /**
+     * Converts an integer into an IPv4 address.
+     *
+     * @param address an integer representing an IP value
+     * @param netmask the CIDR value subnet mask
+     * @return an IP address
+     */
+    public static IpAddress valueOf(int address, int netmask) {
+        return new IpAddress(Version.INET, bytes(address), netmask);
     }
 
     /**
-     * Converts a string in dotted-decimal notation (x.x.x.x) into
-     * an IPv4 address.
+     * Converts a dotted-decimal string (x.x.x.x) into an IPv4 address. The
+     * string can also be in CIDR (slash) notation.
      *
-     * @param address a string representing an IP address, e.g. "10.0.0.1"
+     * @param address a IP address in string form, e.g. "10.0.0.1", "10.0.0.1/24"
      * @return an IP address
      */
     public static IpAddress valueOf(String address) {
-        final String [] parts = address.split("\\.");
-        if (parts.length != INET_LEN) {
+
+        final String [] parts = address.split("\\/");
+        if (parts.length > 2) {
+            throw new IllegalArgumentException("Malformed IP address string; "
+                    + "Addres must take form \"x.x.x.x\" or \"x.x.x.x/y\"");
+        }
+
+        int mask = DEFAULT_MASK;
+        if (parts.length == 2) {
+            mask = Integer.valueOf(parts[1]);
+            if (mask > MAX_INET_MASK) {
+                throw new IllegalArgumentException(
+                        "Value of subnet mask cannot exceed "
+                        + MAX_INET_MASK);
+            }
+        }
+
+        final String [] net = parts[0].split("\\.");
+        if (net.length != INET_LEN) {
             throw new IllegalArgumentException("Malformed IP address string; "
                     + "Addres must have four decimal values separated by dots (.)");
         }
         final byte [] bytes = new byte[INET_LEN];
         for (int i = 0; i < INET_LEN; i++) {
-            bytes[i] = Byte.parseByte(parts[i], 10);
+            bytes[i] = (byte) Short.parseShort(net[i], 10);
         }
-        return new IpAddress(Version.INET, bytes);
+        return new IpAddress(Version.INET, bytes, mask);
     }
 
     /**
@@ -99,34 +165,122 @@ public class IpAddress {
         return address;
     }
 
-    @Override
-    public String toString() {
-        final StringBuilder builder = new StringBuilder();
-        for (final byte b : this.octets) {
-            if (builder.length() > 0) {
-                builder.append(".");
-            }
-            builder.append(String.format("%d", b));
+    /**
+     * Helper for computing the mask value from CIDR.
+     *
+     * @return an integer bitmask
+     */
+    private int mask() {
+        int shift = MAX_INET_MASK - this.netmask;
+        return ((Integer.MAX_VALUE >>> (shift - 1)) << shift);
+    }
+
+    /**
+     * Returns the subnet mask in IpAddress form. The netmask value for
+     * the returned IpAddress is 0, as the address itself is a mask.
+     *
+     * @return the subnet mask
+     */
+    public IpAddress netmask() {
+        return new IpAddress(Version.INET, bytes(mask()));
+    }
+
+    /**
+     * Returns the network portion of this address as an IpAddress.
+     * The netmask of the returned IpAddress is the current mask. If this
+     * address doesn't have a mask, this returns an all-0 IpAddress.
+     *
+     * @return the network address or null
+     */
+    public IpAddress network() {
+        if (netmask == DEFAULT_MASK) {
+            return new IpAddress(version, ANY, DEFAULT_MASK);
         }
-        return builder.toString();
+
+        byte [] net = new byte [4];
+        byte [] mask = bytes(mask());
+        for (int i = 0; i < INET_LEN; i++) {
+             net[i] = (byte) (octets[i] & mask[i]);
+        }
+        return new IpAddress(version, net, netmask);
+    }
+
+    /**
+     * Returns the host portion of the IPAddress, as an IPAddress.
+     * The netmask of the returned IpAddress is the current mask. If this
+     * address doesn't have a mask, this returns a copy of the current
+     * address.
+     *
+     * @return the host address
+     */
+    public IpAddress host() {
+        if (netmask == DEFAULT_MASK) {
+            new IpAddress(version, octets, netmask);
+        }
+
+        byte [] host = new byte [INET_LEN];
+        byte [] mask = bytes(mask());
+        for (int i = 0; i < INET_LEN; i++) {
+             host[i] = (byte) (octets[i] & ~mask[i]);
+        }
+        return new IpAddress(version, host, netmask);
     }
 
     @Override
     public int hashCode() {
-        return Arrays.hashCode(octets);
+        final int prime = 31;
+        int result = 1;
+        result = prime * result + netmask;
+        result = prime * result + Arrays.hashCode(octets);
+        result = prime * result + ((version == null) ? 0 : version.hashCode());
+        return result;
     }
 
     @Override
     public boolean equals(Object obj) {
+        if (this == obj) {
+            return true;
+        }
+        if (obj == null) {
+            return false;
+        }
+        if (getClass() != obj.getClass()) {
+            return false;
+        }
+        IpAddress other = (IpAddress) obj;
+        if (netmask != other.netmask) {
+            return false;
+        }
+        if (!Arrays.equals(octets, other.octets)) {
+            return false;
+        }
+        if (version != other.version) {
+            return false;
+        }
+        return true;
+    }
 
-        if (obj instanceof IpAddress) {
-            IpAddress other = (IpAddress) obj;
-
-            if (this.version.equals(other.version)
-                    && (Arrays.equals(this.octets, other.octets))) {
-                return true;
+    @Override
+    /*
+     * (non-Javadoc)
+     * format is "x.x.x.x" for non-masked (netmask 0) addresses,
+     * and "x.x.x.x/y" for masked addresses.
+     *
+     * @see java.lang.Object#toString()
+     */
+    public String toString() {
+        final StringBuilder builder = new StringBuilder();
+        for (final byte b : this.octets) {
+            if (builder.length() > 0) {
+                builder.append(".");
             }
+            builder.append(String.format("%d", b & 0xff));
         }
-        return false;
+        if (netmask != DEFAULT_MASK) {
+            builder.append("/");
+            builder.append(String.format("%d", netmask));
+        }
+        return builder.toString();
     }
+
 }

 package org.onlab.packet;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
 
 import java.util.Arrays;
 
@@ -11,33 +12,65 @@ import com.google.common.testing.EqualsTester;
 
 public class IPAddressTest {
 
-    private static final byte [] BYTES1 = new byte [] {0x0, 0x0, 0x0, 0xa};
-    private static final byte [] BYTES2 = new byte [] {0x0, 0x0, 0x0, 0xb};
-    private static final int INTVAL1 = 10;
-    private static final int INTVAL2 = 12;
-    private static final String STRVAL = "0.0.0.11";
+    private static final byte [] BYTES1 = new byte [] {0xa, 0x0, 0x0, 0xa};
+    private static final byte [] BYTES2 = new byte [] {0xa, 0x0, 0x0, 0xb};
+    private static final int INTVAL1 = 167772170;
+    private static final int INTVAL2 = 167772171;
+    private static final String STRVAL = "10.0.0.12";
+    private static final int MASK = 16;
 
     @Test
     public void testEquality() {
         IpAddress ip1 = IpAddress.valueOf(BYTES1);
-        IpAddress ip2 = IpAddress.valueOf(BYTES2);
-        IpAddress ip3 = IpAddress.valueOf(INTVAL1);
+        IpAddress ip2 = IpAddress.valueOf(INTVAL1);
+        IpAddress ip3 = IpAddress.valueOf(BYTES2);
         IpAddress ip4 = IpAddress.valueOf(INTVAL2);
         IpAddress ip5 = IpAddress.valueOf(STRVAL);
 
-        new EqualsTester().addEqualityGroup(ip1, ip3)
-        .addEqualityGroup(ip2, ip5)
-        .addEqualityGroup(ip4)
+        new EqualsTester().addEqualityGroup(ip1, ip2)
+        .addEqualityGroup(ip3, ip4)
+        .addEqualityGroup(ip5)
         .testEquals();
+
+        // string conversions
+        IpAddress ip6 = IpAddress.valueOf(BYTES1, MASK);
+        IpAddress ip7 = IpAddress.valueOf("10.0.0.10/16");
+        IpAddress ip8 = IpAddress.valueOf(new byte [] {0xa, 0x0, 0x0, 0xc});
+        assertEquals("incorrect address conversion", ip6, ip7);
+        assertEquals("incorrect address conversion", ip5, ip8);
     }
 
     @Test
     public void basics() {
-        IpAddress ip4 = IpAddress.valueOf(BYTES1);
-        assertEquals("incorrect IP Version", Version.INET, ip4.version());
-        assertEquals("faulty toOctets()", Arrays.equals(
-                new byte [] {0x0, 0x0, 0x0, 0xa}, ip4.toOctets()), true);
-        assertEquals("faulty toInt()", INTVAL1, ip4.toInt());
-        assertEquals("faulty toString()", "0.0.0.10", ip4.toString());
+        IpAddress ip1 = IpAddress.valueOf(BYTES1, MASK);
+        final byte [] bytes = new byte [] {0xa, 0x0, 0x0, 0xa};
+
+        //check fields
+        assertEquals("incorrect IP Version", Version.INET, ip1.version());
+        assertEquals("incorrect netmask", 16, ip1.netmask);
+        assertTrue("faulty toOctets()", Arrays.equals(bytes, ip1.toOctets()));
+        assertEquals("faulty toInt()", INTVAL1, ip1.toInt());
+        assertEquals("faulty toString()", "10.0.0.10/16", ip1.toString());
+    }
+
+    @Test
+    public void netmasks() {
+        // masked
+        IpAddress ip1 = IpAddress.valueOf(BYTES1, MASK);
+
+        IpAddress host = IpAddress.valueOf("0.0.0.10/16");
+        IpAddress network = IpAddress.valueOf("10.0.0.0/16");
+        assertEquals("incorrect host address", host, ip1.host());
+        assertEquals("incorrect network address", network, ip1.network());
+        assertEquals("incorrect netmask", "255.255.0.0", ip1.netmask().toString());
+
+        //unmasked
+        IpAddress ip2 = IpAddress.valueOf(BYTES1);
+        IpAddress umhost = IpAddress.valueOf("10.0.0.10/0");
+        IpAddress umnet = IpAddress.valueOf("0.0.0.0/0");
+        assertEquals("incorrect host address", umhost, ip2.host());
+        assertEquals("incorrect host address", umnet, ip2.network());
+        assertTrue("incorrect netmask",
+                Arrays.equals(IpAddress.ANY, ip2.netmask().toOctets()));
     }
 }