홍길동 / onos

Go to a project
Toggle navigation Toggle navigation pinning
Jonathan Hart
a887ba8a 1 parent 1c6e94f2

Refactored the HostStore to allow multiple MAC addresses bound to a single port 

Change-Id: Icd3b2e483b15486251ac1cca107478a012d1a3e7
Inline Side-by-side
Showing 14 changed files with 152 additions and 222 deletions
...@@ -53,11 +53,13 @@ public class HostToInterfaceAdaptor implements InterfaceService { ...@@ -53,11 +53,13 @@ public class HostToInterfaceAdaptor implements InterfaceService {
53 public Interface getInterface(ConnectPoint connectPoint) { 53 public Interface getInterface(ConnectPoint connectPoint) {
54 checkNotNull(connectPoint); 54 checkNotNull(connectPoint);
55 55
56 - PortAddresses portAddresses = 56 + Set<PortAddresses> portAddresses =
57 hostService.getAddressBindingsForPort(connectPoint); 57 hostService.getAddressBindingsForPort(connectPoint);
58 58
59 - if (!portAddresses.ipAddresses().isEmpty()) { 59 + for (PortAddresses addresses : portAddresses) {
60 - return new Interface(portAddresses); 60 + if (addresses.connectPoint().equals(connectPoint)) {
61 + return new Interface(addresses);
62 + }
61 } 63 }
62 64
63 return null; 65 return null;
......
...@@ -23,6 +23,7 @@ import static org.junit.Assert.assertEquals; ...@@ -23,6 +23,7 @@ import static org.junit.Assert.assertEquals;
23 import static org.junit.Assert.assertNull; 23 import static org.junit.Assert.assertNull;
24 import static org.junit.Assert.assertTrue; 24 import static org.junit.Assert.assertTrue;
25 25
26 +import java.util.Collections;
26 import java.util.Map; 27 import java.util.Map;
27 import java.util.Set; 28 import java.util.Set;
28 29
...@@ -63,10 +64,6 @@ public class HostToInterfaceAdaptorTest { ...@@ -63,10 +64,6 @@ public class HostToInterfaceAdaptorTest {
63 private static final ConnectPoint NON_EXISTENT_CP = new ConnectPoint( 64 private static final ConnectPoint NON_EXISTENT_CP = new ConnectPoint(
64 DeviceId.deviceId("doesnotexist"), PortNumber.portNumber(1)); 65 DeviceId.deviceId("doesnotexist"), PortNumber.portNumber(1));
65 66
66 - private static final PortAddresses DEFAULT_PA = new PortAddresses(
67 - NON_EXISTENT_CP, null, null);
68 -
69 -
70 @Before 67 @Before
71 public void setUp() throws Exception { 68 public void setUp() throws Exception {
72 hostService = createMock(HostService.class); 69 hostService = createMock(HostService.class);
...@@ -123,7 +120,8 @@ public class HostToInterfaceAdaptorTest { ...@@ -123,7 +120,8 @@ public class HostToInterfaceAdaptorTest {
123 MacAddress mac) { 120 MacAddress mac) {
124 PortAddresses pa = new PortAddresses(cp, ipAddresses, mac); 121 PortAddresses pa = new PortAddresses(cp, ipAddresses, mac);
125 portAddresses.add(pa); 122 portAddresses.add(pa);
126 - expect(hostService.getAddressBindingsForPort(cp)).andReturn(pa).anyTimes(); 123 + expect(hostService.getAddressBindingsForPort(cp)).andReturn(
124 + Collections.singleton(pa)).anyTimes();
127 125
128 Interface intf = new Interface(cp, ipAddresses, mac); 126 Interface intf = new Interface(cp, ipAddresses, mac);
129 interfaces.put(cp, intf); 127 interfaces.put(cp, intf);
...@@ -158,7 +156,7 @@ public class HostToInterfaceAdaptorTest { ...@@ -158,7 +156,7 @@ public class HostToInterfaceAdaptorTest {
158 // Try and get an interface for a connect point with no addresses 156 // Try and get an interface for a connect point with no addresses
159 reset(hostService); 157 reset(hostService);
160 expect(hostService.getAddressBindingsForPort(NON_EXISTENT_CP)) 158 expect(hostService.getAddressBindingsForPort(NON_EXISTENT_CP))
161 - .andReturn(DEFAULT_PA).anyTimes(); 159 + .andReturn(Collections.<PortAddresses>emptySet()).anyTimes();
162 replay(hostService); 160 replay(hostService);
163 161
164 assertNull(adaptor.getInterface(NON_EXISTENT_CP)); 162 assertNull(adaptor.getInterface(NON_EXISTENT_CP));
......
...@@ -34,11 +34,7 @@ public interface HostAdminService { ...@@ -34,11 +34,7 @@ public interface HostAdminService {
34 * Binds IP and MAC addresses to the given connection point. 34 * Binds IP and MAC addresses to the given connection point.
35 * <p> 35 * <p>
36 * The addresses are added to the set of addresses already bound to the 36 * The addresses are added to the set of addresses already bound to the
37 - * connection point. If any of the fields in addresses is null, no change 37 + * connection point.
38 - * is made to the corresponding addresses in the store.
39 - * {@link #unbindAddressesFromPort(PortAddresses)} must be use to unbind
40 - * addresses that have previously been bound.
41 - * </p>
42 * 38 *
43 * @param addresses address object containing addresses to add and the port 39 * @param addresses address object containing addresses to add and the port
44 * to add them to 40 * to add them to
......
...@@ -135,7 +135,7 @@ public interface HostService { ...@@ -135,7 +135,7 @@ public interface HostService {
135 * @param connectPoint the connection point to retrieve address bindings for 135 * @param connectPoint the connection point to retrieve address bindings for
136 * @return addresses bound to the port 136 * @return addresses bound to the port
137 */ 137 */
138 - PortAddresses getAddressBindingsForPort(ConnectPoint connectPoint); 138 + Set<PortAddresses> getAddressBindingsForPort(ConnectPoint connectPoint);
139 139
140 /** 140 /**
141 * Adds the specified host listener. 141 * Adds the specified host listener.
......
...@@ -15,6 +15,8 @@ ...@@ -15,6 +15,8 @@
15 */ 15 */
16 package org.onlab.onos.net.host; 16 package org.onlab.onos.net.host;
17 17
18 +import java.util.Set;
19 +
18 import org.onlab.onos.net.ConnectPoint; 20 import org.onlab.onos.net.ConnectPoint;
19 import org.onlab.onos.net.DeviceId; 21 import org.onlab.onos.net.DeviceId;
20 import org.onlab.onos.net.Host; 22 import org.onlab.onos.net.Host;
...@@ -25,8 +27,6 @@ import org.onlab.packet.IpAddress; ...@@ -25,8 +27,6 @@ import org.onlab.packet.IpAddress;
25 import org.onlab.packet.MacAddress; 27 import org.onlab.packet.MacAddress;
26 import org.onlab.packet.VlanId; 28 import org.onlab.packet.VlanId;
27 29
28 -import java.util.Set;
29 -
30 /** 30 /**
31 * Manages inventory of end-station hosts; not intended for direct use. 31 * Manages inventory of end-station hosts; not intended for direct use.
32 */ 32 */
...@@ -153,5 +153,5 @@ public interface HostStore extends Store<HostEvent, HostStoreDelegate> { ...@@ -153,5 +153,5 @@ public interface HostStore extends Store<HostEvent, HostStoreDelegate> {
153 * for 153 * for
154 * @return address information for the connection point 154 * @return address information for the connection point
155 */ 155 */
156 - PortAddresses getAddressBindingsForPort(ConnectPoint connectPoint); 156 + Set<PortAddresses> getAddressBindingsForPort(ConnectPoint connectPoint);
157 } 157 }
......
...@@ -95,7 +95,7 @@ public class HostServiceAdapter implements HostService { ...@@ -95,7 +95,7 @@ public class HostServiceAdapter implements HostService {
95 } 95 }
96 96
97 @Override 97 @Override
98 - public PortAddresses getAddressBindingsForPort(ConnectPoint connectPoint) { 98 + public Set<PortAddresses> getAddressBindingsForPort(ConnectPoint connectPoint) {
99 return null; 99 return null;
100 } 100 }
101 101
......
...@@ -207,7 +207,7 @@ public class HostManager ...@@ -207,7 +207,7 @@ public class HostManager
207 } 207 }
208 208
209 @Override 209 @Override
210 - public PortAddresses getAddressBindingsForPort(ConnectPoint connectPoint) { 210 + public Set<PortAddresses> getAddressBindingsForPort(ConnectPoint connectPoint) {
211 return store.getAddressBindingsForPort(connectPoint); 211 return store.getAddressBindingsForPort(connectPoint);
212 } 212 }
213 213
......
...@@ -175,13 +175,15 @@ public class HostMonitor implements TimerTask { ...@@ -175,13 +175,15 @@ public class HostMonitor implements TimerTask {
175 for (Device device : deviceService.getDevices()) { 175 for (Device device : deviceService.getDevices()) {
176 for (Port port : deviceService.getPorts(device.id())) { 176 for (Port port : deviceService.getPorts(device.id())) {
177 ConnectPoint cp = new ConnectPoint(device.id(), port.number()); 177 ConnectPoint cp = new ConnectPoint(device.id(), port.number());
178 - PortAddresses portAddresses = 178 + Set<PortAddresses> portAddressSet =
179 hostManager.getAddressBindingsForPort(cp); 179 hostManager.getAddressBindingsForPort(cp);
180 180
181 - for (InterfaceIpAddress ia : portAddresses.ipAddresses()) { 181 + for (PortAddresses portAddresses : portAddressSet) {
182 - if (ia.subnetAddress().contains(targetIp)) { 182 + for (InterfaceIpAddress ia : portAddresses.ipAddresses()) {
183 - sendProbe(device.id(), port, targetIp, 183 + if (ia.subnetAddress().contains(targetIp)) {
184 - ia.ipAddress(), portAddresses.mac()); 184 + sendProbe(device.id(), port, targetIp,
185 + ia.ipAddress(), portAddresses.mac());
186 + }
185 } 187 }
186 } 188 }
187 } 189 }
......
...@@ -134,14 +134,16 @@ public class ProxyArpManager implements ProxyArpService { ...@@ -134,14 +134,16 @@ public class ProxyArpManager implements ProxyArpService {
134 IpAddress target = 134 IpAddress target =
135 IpAddress.valueOf(IpAddress.Version.INET, 135 IpAddress.valueOf(IpAddress.Version.INET,
136 arp.getTargetProtocolAddress()); 136 arp.getTargetProtocolAddress());
137 - PortAddresses addresses = 137 + Set<PortAddresses> addressSet =
138 hostService.getAddressBindingsForPort(inPort); 138 hostService.getAddressBindingsForPort(inPort);
139 139
140 - for (InterfaceIpAddress ia : addresses.ipAddresses()) { 140 + for (PortAddresses addresses : addressSet) {
141 - if (ia.ipAddress().equals(target)) { 141 + for (InterfaceIpAddress ia : addresses.ipAddresses()) {
142 - Ethernet arpReply = 142 + if (ia.ipAddress().equals(target)) {
143 - buildArpReply(ia.ipAddress(), addresses.mac(), eth); 143 + Ethernet arpReply =
144 - sendTo(arpReply, inPort); 144 + buildArpReply(ia.ipAddress(), addresses.mac(), eth);
145 + sendTo(arpReply, inPort);
146 + }
145 } 147 }
146 } 148 }
147 return; 149 return;
...@@ -244,7 +246,7 @@ public class ProxyArpManager implements ProxyArpService { ...@@ -244,7 +246,7 @@ public class ProxyArpManager implements ProxyArpService {
244 // TODO: Is this sufficient to identify outside-facing ports: just 246 // TODO: Is this sufficient to identify outside-facing ports: just
245 // having IP addresses on a port? 247 // having IP addresses on a port?
246 // 248 //
247 - return !hostService.getAddressBindingsForPort(port).ipAddresses().isEmpty(); 249 + return !hostService.getAddressBindingsForPort(port).isEmpty();
248 } 250 }
249 251
250 @Override 252 @Override
......
...@@ -234,10 +234,10 @@ public class HostManagerTest { ...@@ -234,10 +234,10 @@ public class HostManagerTest {
234 new PortAddresses(CP1, Sets.newHashSet(IA1, IA2), MAC1); 234 new PortAddresses(CP1, Sets.newHashSet(IA1, IA2), MAC1);
235 235
236 mgr.bindAddressesToPort(add1); 236 mgr.bindAddressesToPort(add1);
237 - PortAddresses storedAddresses = mgr.getAddressBindingsForPort(CP1); 237 + Set<PortAddresses> storedAddresses = mgr.getAddressBindingsForPort(CP1);
238 238
239 - assertTrue(add1.ipAddresses().equals(storedAddresses.ipAddresses())); 239 + assertEquals(1, storedAddresses.size());
240 - assertTrue(add1.mac().equals(storedAddresses.mac())); 240 + assertTrue(storedAddresses.contains(add1));
241 241
242 // Add some more addresses and check that they're added correctly 242 // Add some more addresses and check that they're added correctly
243 PortAddresses add2 = 243 PortAddresses add2 =
...@@ -246,18 +246,19 @@ public class HostManagerTest { ...@@ -246,18 +246,19 @@ public class HostManagerTest {
246 mgr.bindAddressesToPort(add2); 246 mgr.bindAddressesToPort(add2);
247 storedAddresses = mgr.getAddressBindingsForPort(CP1); 247 storedAddresses = mgr.getAddressBindingsForPort(CP1);
248 248
249 - assertTrue(storedAddresses.ipAddresses().equals( 249 + assertEquals(2, storedAddresses.size());
250 - Sets.newHashSet(IA1, IA2, IA3))); 250 + assertTrue(storedAddresses.contains(add1));
251 - assertTrue(storedAddresses.mac().equals(MAC1)); 251 + assertTrue(storedAddresses.contains(add2));
252 252
253 PortAddresses add3 = new PortAddresses(CP1, null, MAC2); 253 PortAddresses add3 = new PortAddresses(CP1, null, MAC2);
254 254
255 mgr.bindAddressesToPort(add3); 255 mgr.bindAddressesToPort(add3);
256 storedAddresses = mgr.getAddressBindingsForPort(CP1); 256 storedAddresses = mgr.getAddressBindingsForPort(CP1);
257 257
258 - assertTrue(storedAddresses.ipAddresses().equals( 258 + assertEquals(3, storedAddresses.size());
259 - Sets.newHashSet(IA1, IA2, IA3))); 259 + assertTrue(storedAddresses.contains(add1));
260 - assertTrue(storedAddresses.mac().equals(MAC2)); 260 + assertTrue(storedAddresses.contains(add2));
261 + assertTrue(storedAddresses.contains(add3));
261 } 262 }
262 263
263 @Test 264 @Test
...@@ -266,10 +267,10 @@ public class HostManagerTest { ...@@ -266,10 +267,10 @@ public class HostManagerTest {
266 new PortAddresses(CP1, Sets.newHashSet(IA1, IA2), MAC1); 267 new PortAddresses(CP1, Sets.newHashSet(IA1, IA2), MAC1);
267 268
268 mgr.bindAddressesToPort(add1); 269 mgr.bindAddressesToPort(add1);
269 - PortAddresses storedAddresses = mgr.getAddressBindingsForPort(CP1); 270 + Set<PortAddresses> storedAddresses = mgr.getAddressBindingsForPort(CP1);
270 271
271 - assertTrue(storedAddresses.ipAddresses().size() == 2); 272 + assertEquals(1, storedAddresses.size());
272 - assertNotNull(storedAddresses.mac()); 273 + assertTrue(storedAddresses.contains(add1));
273 274
274 PortAddresses rem1 = 275 PortAddresses rem1 =
275 new PortAddresses(CP1, Sets.newHashSet(IA1), null); 276 new PortAddresses(CP1, Sets.newHashSet(IA1), null);
...@@ -277,25 +278,15 @@ public class HostManagerTest { ...@@ -277,25 +278,15 @@ public class HostManagerTest {
277 mgr.unbindAddressesFromPort(rem1); 278 mgr.unbindAddressesFromPort(rem1);
278 storedAddresses = mgr.getAddressBindingsForPort(CP1); 279 storedAddresses = mgr.getAddressBindingsForPort(CP1);
279 280
280 - assertTrue(storedAddresses.ipAddresses().equals(Sets.newHashSet(IA2))); 281 + // It shouldn't have been removed because it didn't match the originally
281 - assertTrue(storedAddresses.mac().equals(MAC1)); 282 + // submitted address object
283 + assertEquals(1, storedAddresses.size());
284 + assertTrue(storedAddresses.contains(add1));
282 285
283 - PortAddresses rem2 = new PortAddresses(CP1, null, MAC1); 286 + mgr.unbindAddressesFromPort(add1);
284 -
285 - mgr.unbindAddressesFromPort(rem2);
286 storedAddresses = mgr.getAddressBindingsForPort(CP1); 287 storedAddresses = mgr.getAddressBindingsForPort(CP1);
287 288
288 - assertTrue(storedAddresses.ipAddresses().equals(Sets.newHashSet(IA2))); 289 + assertTrue(storedAddresses.isEmpty());
289 - assertNull(storedAddresses.mac());
290 -
291 - PortAddresses rem3 =
292 - new PortAddresses(CP1, Sets.newHashSet(IA2), MAC1);
293 -
294 - mgr.unbindAddressesFromPort(rem3);
295 - storedAddresses = mgr.getAddressBindingsForPort(CP1);
296 -
297 - assertTrue(storedAddresses.ipAddresses().isEmpty());
298 - assertNull(storedAddresses.mac());
299 } 290 }
300 291
301 @Test 292 @Test
...@@ -304,16 +295,15 @@ public class HostManagerTest { ...@@ -304,16 +295,15 @@ public class HostManagerTest {
304 new PortAddresses(CP1, Sets.newHashSet(IA1, IA2), MAC1); 295 new PortAddresses(CP1, Sets.newHashSet(IA1, IA2), MAC1);
305 296
306 mgr.bindAddressesToPort(add1); 297 mgr.bindAddressesToPort(add1);
307 - PortAddresses storedAddresses = mgr.getAddressBindingsForPort(CP1); 298 + Set<PortAddresses> storedAddresses = mgr.getAddressBindingsForPort(CP1);
308 299
309 - assertTrue(storedAddresses.ipAddresses().size() == 2); 300 + assertEquals(1, storedAddresses.size());
310 - assertNotNull(storedAddresses.mac()); 301 + assertTrue(storedAddresses.contains(add1));
311 302
312 mgr.clearAddresses(CP1); 303 mgr.clearAddresses(CP1);
313 storedAddresses = mgr.getAddressBindingsForPort(CP1); 304 storedAddresses = mgr.getAddressBindingsForPort(CP1);
314 305
315 - assertTrue(storedAddresses.ipAddresses().isEmpty()); 306 + assertTrue(storedAddresses.isEmpty());
316 - assertNull(storedAddresses.mac());
317 } 307 }
318 308
319 @Test 309 @Test
...@@ -322,12 +312,10 @@ public class HostManagerTest { ...@@ -322,12 +312,10 @@ public class HostManagerTest {
322 new PortAddresses(CP1, Sets.newHashSet(IA1, IA2), MAC1); 312 new PortAddresses(CP1, Sets.newHashSet(IA1, IA2), MAC1);
323 313
324 mgr.bindAddressesToPort(add1); 314 mgr.bindAddressesToPort(add1);
325 - PortAddresses storedAddresses = mgr.getAddressBindingsForPort(CP1); 315 + Set<PortAddresses> storedAddresses = mgr.getAddressBindingsForPort(CP1);
326 316
327 - assertTrue(storedAddresses.connectPoint().equals(CP1)); 317 + assertEquals(1, storedAddresses.size());
328 - assertTrue(storedAddresses.ipAddresses().equals( 318 + assertTrue(storedAddresses.contains(add1));
329 - Sets.newHashSet(IA1, IA2)));
330 - assertTrue(storedAddresses.mac().equals(MAC1));
331 } 319 }
332 320
333 @Test 321 @Test
......
...@@ -20,7 +20,9 @@ import static org.easymock.EasyMock.expect; ...@@ -20,7 +20,9 @@ import static org.easymock.EasyMock.expect;
20 import static org.easymock.EasyMock.expectLastCall; 20 import static org.easymock.EasyMock.expectLastCall;
21 import static org.easymock.EasyMock.replay; 21 import static org.easymock.EasyMock.replay;
22 import static org.easymock.EasyMock.verify; 22 import static org.easymock.EasyMock.verify;
23 -import static org.junit.Assert.*; 23 +import static org.junit.Assert.assertArrayEquals;
24 +import static org.junit.Assert.assertEquals;
25 +import static org.junit.Assert.assertTrue;
24 26
25 import java.util.ArrayList; 27 import java.util.ArrayList;
26 import java.util.Collections; 28 import java.util.Collections;
...@@ -130,7 +132,7 @@ public class HostMonitorTest { ...@@ -130,7 +132,7 @@ public class HostMonitorTest {
130 expect(hostManager.getHostsByIp(TARGET_IP_ADDR)) 132 expect(hostManager.getHostsByIp(TARGET_IP_ADDR))
131 .andReturn(Collections.<Host>emptySet()).anyTimes(); 133 .andReturn(Collections.<Host>emptySet()).anyTimes();
132 expect(hostManager.getAddressBindingsForPort(cp)) 134 expect(hostManager.getAddressBindingsForPort(cp))
133 - .andReturn(pa).anyTimes(); 135 + .andReturn(Collections.singleton(pa)).anyTimes();
134 replay(hostManager); 136 replay(hostManager);
135 137
136 TestPacketService packetService = new TestPacketService(); 138 TestPacketService packetService = new TestPacketService();
......
...@@ -19,7 +19,10 @@ import static org.easymock.EasyMock.anyObject; ...@@ -19,7 +19,10 @@ import static org.easymock.EasyMock.anyObject;
19 import static org.easymock.EasyMock.createMock; 19 import static org.easymock.EasyMock.createMock;
20 import static org.easymock.EasyMock.expect; 20 import static org.easymock.EasyMock.expect;
21 import static org.easymock.EasyMock.replay; 21 import static org.easymock.EasyMock.replay;
22 -import static org.junit.Assert.*; 22 +import static org.junit.Assert.assertArrayEquals;
23 +import static org.junit.Assert.assertEquals;
24 +import static org.junit.Assert.assertFalse;
25 +import static org.junit.Assert.assertTrue;
23 26
24 import java.util.ArrayList; 27 import java.util.ArrayList;
25 import java.util.Collections; 28 import java.util.Collections;
...@@ -207,13 +210,18 @@ public class ProxyArpManagerTest { ...@@ -207,13 +210,18 @@ public class ProxyArpManagerTest {
207 IpAddress addr2 = IpAddress.valueOf("10.0." + (2 * i) + ".1"); 210 IpAddress addr2 = IpAddress.valueOf("10.0." + (2 * i) + ".1");
208 InterfaceIpAddress ia1 = new InterfaceIpAddress(addr1, prefix1); 211 InterfaceIpAddress ia1 = new InterfaceIpAddress(addr1, prefix1);
209 InterfaceIpAddress ia2 = new InterfaceIpAddress(addr2, prefix2); 212 InterfaceIpAddress ia2 = new InterfaceIpAddress(addr2, prefix2);
210 - PortAddresses pa = 213 + PortAddresses pa1 =
211 - new PortAddresses(cp, Sets.newHashSet(ia1, ia2), 214 + new PortAddresses(cp, Sets.newHashSet(ia1),
212 - MacAddress.valueOf(i)); 215 + MacAddress.valueOf(2 * i - 1));
213 - addresses.add(pa); 216 + PortAddresses pa2 =
217 + new PortAddresses(cp, Sets.newHashSet(ia2),
218 + MacAddress.valueOf(2 * i));
219 +
220 + addresses.add(pa1);
221 + addresses.add(pa2);
214 222
215 expect(hostService.getAddressBindingsForPort(cp)) 223 expect(hostService.getAddressBindingsForPort(cp))
216 - .andReturn(pa).anyTimes(); 224 + .andReturn(Sets.newHashSet(pa1, pa2)).anyTimes();
217 } 225 }
218 226
219 expect(hostService.getAddressBindings()).andReturn(addresses).anyTimes(); 227 expect(hostService.getAddressBindings()).andReturn(addresses).anyTimes();
...@@ -222,7 +230,7 @@ public class ProxyArpManagerTest { ...@@ -222,7 +230,7 @@ public class ProxyArpManagerTest {
222 ConnectPoint cp = new ConnectPoint(getDeviceId(i + NUM_ADDRESS_PORTS), 230 ConnectPoint cp = new ConnectPoint(getDeviceId(i + NUM_ADDRESS_PORTS),
223 P1); 231 P1);
224 expect(hostService.getAddressBindingsForPort(cp)) 232 expect(hostService.getAddressBindingsForPort(cp))
225 - .andReturn(new PortAddresses(cp, null, null)).anyTimes(); 233 + .andReturn(Collections.<PortAddresses>emptySet()).anyTimes();
226 } 234 }
227 } 235 }
228 236
...@@ -339,7 +347,8 @@ public class ProxyArpManagerTest { ...@@ -339,7 +347,8 @@ public class ProxyArpManagerTest {
339 IpAddress theirIp = IpAddress.valueOf("10.0.1.254"); 347 IpAddress theirIp = IpAddress.valueOf("10.0.1.254");
340 IpAddress ourFirstIp = IpAddress.valueOf("10.0.1.1"); 348 IpAddress ourFirstIp = IpAddress.valueOf("10.0.1.1");
341 IpAddress ourSecondIp = IpAddress.valueOf("10.0.2.1"); 349 IpAddress ourSecondIp = IpAddress.valueOf("10.0.2.1");
342 - MacAddress ourMac = MacAddress.valueOf(1L); 350 + MacAddress firstMac = MacAddress.valueOf(1L);
351 + MacAddress secondMac = MacAddress.valueOf(2L);
343 352
344 Host requestor = new DefaultHost(PID, HID2, MAC2, VLAN1, LOC1, 353 Host requestor = new DefaultHost(PID, HID2, MAC2, VLAN1, LOC1,
345 Collections.singleton(theirIp)); 354 Collections.singleton(theirIp));
...@@ -352,7 +361,7 @@ public class ProxyArpManagerTest { ...@@ -352,7 +361,7 @@ public class ProxyArpManagerTest {
352 proxyArp.reply(arpRequest, LOC1); 361 proxyArp.reply(arpRequest, LOC1);
353 362
354 assertEquals(1, packetService.packets.size()); 363 assertEquals(1, packetService.packets.size());
355 - Ethernet arpReply = buildArp(ARP.OP_REPLY, ourMac, MAC2, ourFirstIp, theirIp); 364 + Ethernet arpReply = buildArp(ARP.OP_REPLY, firstMac, MAC2, ourFirstIp, theirIp);
356 verifyPacketOut(arpReply, LOC1, packetService.packets.get(0)); 365 verifyPacketOut(arpReply, LOC1, packetService.packets.get(0));
357 366
358 // Test a request for the second address on that port 367 // Test a request for the second address on that port
...@@ -362,7 +371,7 @@ public class ProxyArpManagerTest { ...@@ -362,7 +371,7 @@ public class ProxyArpManagerTest {
362 proxyArp.reply(arpRequest, LOC1); 371 proxyArp.reply(arpRequest, LOC1);
363 372
364 assertEquals(1, packetService.packets.size()); 373 assertEquals(1, packetService.packets.size());
365 - arpReply = buildArp(ARP.OP_REPLY, ourMac, MAC2, ourSecondIp, theirIp); 374 + arpReply = buildArp(ARP.OP_REPLY, secondMac, MAC2, ourSecondIp, theirIp);
366 verifyPacketOut(arpReply, LOC1, packetService.packets.get(0)); 375 verifyPacketOut(arpReply, LOC1, packetService.packets.get(0));
367 } 376 }
368 377
......
...@@ -15,12 +15,25 @@ ...@@ -15,12 +15,25 @@
15 */ 15 */
16 package org.onlab.onos.store.host.impl; 16 package org.onlab.onos.store.host.impl;
17 17
18 -import com.google.common.collect.FluentIterable; 18 +import static java.util.concurrent.Executors.newSingleThreadScheduledExecutor;
19 -import com.google.common.collect.HashMultimap; 19 +import static org.onlab.onos.cluster.ControllerNodeToNodeId.toNodeId;
20 -import com.google.common.collect.ImmutableList; 20 +import static org.onlab.onos.net.host.HostEvent.Type.HOST_ADDED;
21 -import com.google.common.collect.ImmutableSet; 21 +import static org.onlab.onos.net.host.HostEvent.Type.HOST_MOVED;
22 -import com.google.common.collect.Multimap; 22 +import static org.onlab.onos.net.host.HostEvent.Type.HOST_REMOVED;
23 -import com.google.common.collect.Sets; 23 +import static org.onlab.onos.net.host.HostEvent.Type.HOST_UPDATED;
24 +import static org.onlab.util.Tools.namedThreads;
25 +import static org.slf4j.LoggerFactory.getLogger;
26 +
27 +import java.io.IOException;
28 +import java.util.Collections;
29 +import java.util.HashMap;
30 +import java.util.HashSet;
31 +import java.util.Map;
32 +import java.util.Map.Entry;
33 +import java.util.Set;
34 +import java.util.concurrent.ConcurrentHashMap;
35 +import java.util.concurrent.ScheduledExecutorService;
36 +import java.util.concurrent.TimeUnit;
24 37
25 import org.apache.commons.lang3.RandomUtils; 38 import org.apache.commons.lang3.RandomUtils;
26 import org.apache.felix.scr.annotations.Activate; 39 import org.apache.felix.scr.annotations.Activate;
...@@ -45,7 +58,6 @@ import org.onlab.onos.net.host.HostDescription; ...@@ -45,7 +58,6 @@ import org.onlab.onos.net.host.HostDescription;
45 import org.onlab.onos.net.host.HostEvent; 58 import org.onlab.onos.net.host.HostEvent;
46 import org.onlab.onos.net.host.HostStore; 59 import org.onlab.onos.net.host.HostStore;
47 import org.onlab.onos.net.host.HostStoreDelegate; 60 import org.onlab.onos.net.host.HostStoreDelegate;
48 -import org.onlab.onos.net.host.InterfaceIpAddress;
49 import org.onlab.onos.net.host.PortAddresses; 61 import org.onlab.onos.net.host.PortAddresses;
50 import org.onlab.onos.net.provider.ProviderId; 62 import org.onlab.onos.net.provider.ProviderId;
51 import org.onlab.onos.store.AbstractStore; 63 import org.onlab.onos.store.AbstractStore;
...@@ -63,21 +75,13 @@ import org.onlab.packet.VlanId; ...@@ -63,21 +75,13 @@ import org.onlab.packet.VlanId;
63 import org.onlab.util.KryoNamespace; 75 import org.onlab.util.KryoNamespace;
64 import org.slf4j.Logger; 76 import org.slf4j.Logger;
65 77
66 -import java.io.IOException; 78 +import com.google.common.collect.FluentIterable;
67 -import java.util.HashMap; 79 +import com.google.common.collect.HashMultimap;
68 -import java.util.HashSet; 80 +import com.google.common.collect.ImmutableList;
69 -import java.util.Map; 81 +import com.google.common.collect.ImmutableSet;
70 -import java.util.Set; 82 +import com.google.common.collect.Multimap;
71 -import java.util.Map.Entry; 83 +import com.google.common.collect.Multimaps;
72 -import java.util.concurrent.ConcurrentHashMap; 84 +import com.google.common.collect.SetMultimap;
73 -import java.util.concurrent.ScheduledExecutorService;
74 -import java.util.concurrent.TimeUnit;
75 -
76 -import static java.util.concurrent.Executors.newSingleThreadScheduledExecutor;
77 -import static org.onlab.onos.cluster.ControllerNodeToNodeId.toNodeId;
78 -import static org.onlab.onos.net.host.HostEvent.Type.*;
79 -import static org.onlab.util.Tools.namedThreads;
80 -import static org.slf4j.LoggerFactory.getLogger;
81 85
82 //TODO: multi-provider, annotation not supported. 86 //TODO: multi-provider, annotation not supported.
83 /** 87 /**
...@@ -100,8 +104,9 @@ public class GossipHostStore ...@@ -100,8 +104,9 @@ public class GossipHostStore
100 // Hosts tracked by their location 104 // Hosts tracked by their location
101 private final Multimap<ConnectPoint, Host> locations = HashMultimap.create(); 105 private final Multimap<ConnectPoint, Host> locations = HashMultimap.create();
102 106
103 - private final Map<ConnectPoint, PortAddresses> portAddresses = 107 + private final SetMultimap<ConnectPoint, PortAddresses> portAddresses =
104 - new ConcurrentHashMap<>(); 108 + Multimaps.synchronizedSetMultimap(
109 + HashMultimap.<ConnectPoint, PortAddresses>create());
105 110
106 @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY) 111 @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
107 protected HostClockService hostClockService; 112 protected HostClockService hostClockService;
...@@ -343,77 +348,37 @@ public class GossipHostStore ...@@ -343,77 +348,37 @@ public class GossipHostStore
343 348
344 @Override 349 @Override
345 public void updateAddressBindings(PortAddresses addresses) { 350 public void updateAddressBindings(PortAddresses addresses) {
346 - synchronized (portAddresses) { 351 + portAddresses.put(addresses.connectPoint(), addresses);
347 - PortAddresses existing = portAddresses.get(addresses.connectPoint());
348 - if (existing == null) {
349 - portAddresses.put(addresses.connectPoint(), addresses);
350 - } else {
351 - Set<InterfaceIpAddress> union =
352 - Sets.union(existing.ipAddresses(),
353 - addresses.ipAddresses()).immutableCopy();
354 -
355 - MacAddress newMac = (addresses.mac() == null) ? existing.mac()
356 - : addresses.mac();
357 -
358 - PortAddresses newAddresses =
359 - new PortAddresses(addresses.connectPoint(), union, newMac);
360 -
361 - portAddresses.put(newAddresses.connectPoint(), newAddresses);
362 - }
363 - }
364 } 352 }
365 353
366 @Override 354 @Override
367 public void removeAddressBindings(PortAddresses addresses) { 355 public void removeAddressBindings(PortAddresses addresses) {
368 - synchronized (portAddresses) { 356 + portAddresses.remove(addresses.connectPoint(), addresses);
369 - PortAddresses existing = portAddresses.get(addresses.connectPoint());
370 - if (existing != null) {
371 - Set<InterfaceIpAddress> difference =
372 - Sets.difference(existing.ipAddresses(),
373 - addresses.ipAddresses()).immutableCopy();
374 -
375 - // If they removed the existing mac, set the new mac to null.
376 - // Otherwise, keep the existing mac.
377 - MacAddress newMac = existing.mac();
378 - if (addresses.mac() != null && addresses.mac().equals(existing.mac())) {
379 - newMac = null;
380 - }
381 -
382 - PortAddresses newAddresses =
383 - new PortAddresses(addresses.connectPoint(), difference, newMac);
384 -
385 - portAddresses.put(newAddresses.connectPoint(), newAddresses);
386 - }
387 - }
388 } 357 }
389 358
390 @Override 359 @Override
391 public void clearAddressBindings(ConnectPoint connectPoint) { 360 public void clearAddressBindings(ConnectPoint connectPoint) {
392 - synchronized (portAddresses) { 361 + portAddresses.removeAll(connectPoint);
393 - portAddresses.remove(connectPoint);
394 - }
395 } 362 }
396 363
397 @Override 364 @Override
398 public Set<PortAddresses> getAddressBindings() { 365 public Set<PortAddresses> getAddressBindings() {
399 synchronized (portAddresses) { 366 synchronized (portAddresses) {
400 - return new HashSet<>(portAddresses.values()); 367 + return ImmutableSet.copyOf(portAddresses.values());
401 } 368 }
402 } 369 }
403 370
404 @Override 371 @Override
405 - public PortAddresses getAddressBindingsForPort(ConnectPoint connectPoint) { 372 + public Set<PortAddresses> getAddressBindingsForPort(ConnectPoint connectPoint) {
406 - PortAddresses addresses;
407 -
408 synchronized (portAddresses) { 373 synchronized (portAddresses) {
409 - addresses = portAddresses.get(connectPoint); 374 + Set<PortAddresses> addresses = portAddresses.get(connectPoint);
410 - }
411 375
412 - if (addresses == null) { 376 + if (addresses == null) {
413 - addresses = new PortAddresses(connectPoint, null, null); 377 + return Collections.emptySet();
378 + } else {
379 + return ImmutableSet.copyOf(addresses);
380 + }
414 } 381 }
415 -
416 - return addresses;
417 } 382 }
418 383
419 // Auxiliary extension to allow location to mutate. 384 // Auxiliary extension to allow location to mutate.
......
...@@ -15,10 +15,18 @@ ...@@ -15,10 +15,18 @@
15 */ 15 */
16 package org.onlab.onos.store.trivial.impl; 16 package org.onlab.onos.store.trivial.impl;
17 17
18 -import com.google.common.collect.HashMultimap; 18 +import static org.onlab.onos.net.host.HostEvent.Type.HOST_ADDED;
19 -import com.google.common.collect.ImmutableSet; 19 +import static org.onlab.onos.net.host.HostEvent.Type.HOST_MOVED;
20 -import com.google.common.collect.Multimap; 20 +import static org.onlab.onos.net.host.HostEvent.Type.HOST_REMOVED;
21 -import com.google.common.collect.Sets; 21 +import static org.onlab.onos.net.host.HostEvent.Type.HOST_UPDATED;
22 +import static org.slf4j.LoggerFactory.getLogger;
23 +
24 +import java.util.Collections;
25 +import java.util.HashSet;
26 +import java.util.Map;
27 +import java.util.Set;
28 +import java.util.concurrent.ConcurrentHashMap;
29 +
22 import org.apache.felix.scr.annotations.Activate; 30 import org.apache.felix.scr.annotations.Activate;
23 import org.apache.felix.scr.annotations.Component; 31 import org.apache.felix.scr.annotations.Component;
24 import org.apache.felix.scr.annotations.Deactivate; 32 import org.apache.felix.scr.annotations.Deactivate;
...@@ -34,7 +42,6 @@ import org.onlab.onos.net.host.HostDescription; ...@@ -34,7 +42,6 @@ import org.onlab.onos.net.host.HostDescription;
34 import org.onlab.onos.net.host.HostEvent; 42 import org.onlab.onos.net.host.HostEvent;
35 import org.onlab.onos.net.host.HostStore; 43 import org.onlab.onos.net.host.HostStore;
36 import org.onlab.onos.net.host.HostStoreDelegate; 44 import org.onlab.onos.net.host.HostStoreDelegate;
37 -import org.onlab.onos.net.host.InterfaceIpAddress;
38 import org.onlab.onos.net.host.PortAddresses; 45 import org.onlab.onos.net.host.PortAddresses;
39 import org.onlab.onos.net.provider.ProviderId; 46 import org.onlab.onos.net.provider.ProviderId;
40 import org.onlab.onos.store.AbstractStore; 47 import org.onlab.onos.store.AbstractStore;
...@@ -43,13 +50,11 @@ import org.onlab.packet.MacAddress; ...@@ -43,13 +50,11 @@ import org.onlab.packet.MacAddress;
43 import org.onlab.packet.VlanId; 50 import org.onlab.packet.VlanId;
44 import org.slf4j.Logger; 51 import org.slf4j.Logger;
45 52
46 -import java.util.HashSet; 53 +import com.google.common.collect.HashMultimap;
47 -import java.util.Map; 54 +import com.google.common.collect.ImmutableSet;
48 -import java.util.Set; 55 +import com.google.common.collect.Multimap;
49 -import java.util.concurrent.ConcurrentHashMap; 56 +import com.google.common.collect.Multimaps;
50 - 57 +import com.google.common.collect.SetMultimap;
51 -import static org.onlab.onos.net.host.HostEvent.Type.*;
52 -import static org.slf4j.LoggerFactory.getLogger;
53 58
54 // TODO: multi-provider, annotation not supported. 59 // TODO: multi-provider, annotation not supported.
55 /** 60 /**
...@@ -70,8 +75,9 @@ public class SimpleHostStore ...@@ -70,8 +75,9 @@ public class SimpleHostStore
70 // Hosts tracked by their location 75 // Hosts tracked by their location
71 private final Multimap<ConnectPoint, Host> locations = HashMultimap.create(); 76 private final Multimap<ConnectPoint, Host> locations = HashMultimap.create();
72 77
73 - private final Map<ConnectPoint, PortAddresses> portAddresses = 78 + private final SetMultimap<ConnectPoint, PortAddresses> portAddresses =
74 - new ConcurrentHashMap<>(); 79 + Multimaps.synchronizedSetMultimap(
80 + HashMultimap.<ConnectPoint, PortAddresses>create());
75 81
76 @Activate 82 @Activate
77 public void activate() { 83 public void activate() {
...@@ -213,77 +219,37 @@ public class SimpleHostStore ...@@ -213,77 +219,37 @@ public class SimpleHostStore
213 219
214 @Override 220 @Override
215 public void updateAddressBindings(PortAddresses addresses) { 221 public void updateAddressBindings(PortAddresses addresses) {
216 - synchronized (portAddresses) { 222 + portAddresses.put(addresses.connectPoint(), addresses);
217 - PortAddresses existing = portAddresses.get(addresses.connectPoint());
218 - if (existing == null) {
219 - portAddresses.put(addresses.connectPoint(), addresses);
220 - } else {
221 - Set<InterfaceIpAddress> union =
222 - Sets.union(existing.ipAddresses(),
223 - addresses.ipAddresses()).immutableCopy();
224 -
225 - MacAddress newMac = (addresses.mac() == null) ? existing.mac()
226 - : addresses.mac();
227 -
228 - PortAddresses newAddresses =
229 - new PortAddresses(addresses.connectPoint(), union, newMac);
230 -
231 - portAddresses.put(newAddresses.connectPoint(), newAddresses);
232 - }
233 - }
234 } 223 }
235 224
236 @Override 225 @Override
237 public void removeAddressBindings(PortAddresses addresses) { 226 public void removeAddressBindings(PortAddresses addresses) {
238 - synchronized (portAddresses) { 227 + portAddresses.remove(addresses.connectPoint(), addresses);
239 - PortAddresses existing = portAddresses.get(addresses.connectPoint());
240 - if (existing != null) {
241 - Set<InterfaceIpAddress> difference =
242 - Sets.difference(existing.ipAddresses(),
243 - addresses.ipAddresses()).immutableCopy();
244 -
245 - // If they removed the existing mac, set the new mac to null.
246 - // Otherwise, keep the existing mac.
247 - MacAddress newMac = existing.mac();
248 - if (addresses.mac() != null && addresses.mac().equals(existing.mac())) {
249 - newMac = null;
250 - }
251 -
252 - PortAddresses newAddresses =
253 - new PortAddresses(addresses.connectPoint(), difference, newMac);
254 -
255 - portAddresses.put(newAddresses.connectPoint(), newAddresses);
256 - }
257 - }
258 } 228 }
259 229
260 @Override 230 @Override
261 public void clearAddressBindings(ConnectPoint connectPoint) { 231 public void clearAddressBindings(ConnectPoint connectPoint) {
262 - synchronized (portAddresses) { 232 + portAddresses.removeAll(connectPoint);
263 - portAddresses.remove(connectPoint);
264 - }
265 } 233 }
266 234
267 @Override 235 @Override
268 public Set<PortAddresses> getAddressBindings() { 236 public Set<PortAddresses> getAddressBindings() {
269 synchronized (portAddresses) { 237 synchronized (portAddresses) {
270 - return new HashSet<>(portAddresses.values()); 238 + return ImmutableSet.copyOf(portAddresses.values());
271 } 239 }
272 } 240 }
273 241
274 @Override 242 @Override
275 - public PortAddresses getAddressBindingsForPort(ConnectPoint connectPoint) { 243 + public Set<PortAddresses> getAddressBindingsForPort(ConnectPoint connectPoint) {
276 - PortAddresses addresses;
277 -
278 synchronized (portAddresses) { 244 synchronized (portAddresses) {
279 - addresses = portAddresses.get(connectPoint); 245 + Set<PortAddresses> addresses = portAddresses.get(connectPoint);
280 - }
281 246
282 - if (addresses == null) { 247 + if (addresses == null) {
283 - addresses = new PortAddresses(connectPoint, null, null); 248 + return Collections.emptySet();
249 + } else {
250 + return ImmutableSet.copyOf(addresses);
251 + }
284 } 252 }
285 -
286 - return addresses;
287 } 253 }
288 254
289 // Auxiliary extension to allow location to mutate. 255 // Auxiliary extension to allow location to mutate.
......