Jonathan Hart
Committed by Gerrit Code Review

ProxyArp: Reply directly when we know an external target host

Change-Id: I38773dcdcae05506c678c2006d1f63306af6b383
...@@ -146,47 +146,25 @@ public class ProxyArpManager implements ProxyArpService { ...@@ -146,47 +146,25 @@ public class ProxyArpManager implements ProxyArpService {
146 146
147 VlanId vlan = VlanId.vlanId(eth.getVlanID()); 147 VlanId vlan = VlanId.vlanId(eth.getVlanID());
148 148
149 - // If the request came from outside the network, only reply if it was
150 - // for one of our external addresses.
151 if (isOutsidePort(inPort)) { 149 if (isOutsidePort(inPort)) {
150 + // If the request came from outside the network, only reply if it was
151 + // for one of our external addresses.
152 Set<PortAddresses> addressSet = 152 Set<PortAddresses> addressSet =
153 - hostService.getAddressBindingsForPort(inPort); 153 + hostService.getAddressBindingsForPort(inPort);
154 154
155 for (PortAddresses addresses : addressSet) { 155 for (PortAddresses addresses : addressSet) {
156 for (InterfaceIpAddress ia : addresses.ipAddresses()) { 156 for (InterfaceIpAddress ia : addresses.ipAddresses()) {
157 if (ia.ipAddress().equals(targetAddress)) { 157 if (ia.ipAddress().equals(targetAddress)) {
158 Ethernet arpReply = 158 Ethernet arpReply =
159 - buildArpReply(targetAddress, addresses.mac(), eth); 159 + buildArpReply(targetAddress, addresses.mac(), eth);
160 sendTo(arpReply, inPort); 160 sendTo(arpReply, inPort);
161 } 161 }
162 } 162 }
163 } 163 }
164 return; 164 return;
165 - } else {
166 - // If the source address matches one of our external addresses
167 - // it could be a request from an internal host to an external
168 - // address. Forward it over to the correct ports.
169 - Ip4Address source =
170 - Ip4Address.valueOf(arp.getSenderProtocolAddress());
171 - Set<PortAddresses> sourceAddresses = findPortsInSubnet(source);
172 - boolean matched = false;
173 - for (PortAddresses pa : sourceAddresses) {
174 - for (InterfaceIpAddress ia : pa.ipAddresses()) {
175 - if (ia.ipAddress().equals(source) &&
176 - pa.vlan().equals(vlan)) {
177 - matched = true;
178 - sendTo(eth, pa.connectPoint());
179 - break;
180 - }
181 - }
182 - }
183 -
184 - if (matched) {
185 - return;
186 - }
187 } 165 }
188 166
189 - // Continue with normal proxy ARP case 167 + // See if we have the target host in the host store
190 168
191 Set<Host> hosts = hostService.getHostsByIp(targetAddress); 169 Set<Host> hosts = hostService.getHostsByIp(targetAddress);
192 170
...@@ -201,20 +179,41 @@ public class ProxyArpManager implements ProxyArpService { ...@@ -201,20 +179,41 @@ public class ProxyArpManager implements ProxyArpService {
201 } 179 }
202 } 180 }
203 181
204 - if (src == null || dst == null) { 182 + if (src != null && dst != null) {
205 - // 183 + // We know the target host so we can respond
206 - // The request couldn't be resolved. 184 + Ethernet arpReply = buildArpReply(targetAddress, dst.mac(), eth);
207 - // Flood the request on all ports except the incoming ports. 185 + sendTo(arpReply, inPort);
208 - // 186 + return;
209 - flood(eth, inPort); 187 + }
188 +
189 + // If the source address matches one of our external addresses
190 + // it could be a request from an internal host to an external
191 + // address. Forward it over to the correct port.
192 + Ip4Address source =
193 + Ip4Address.valueOf(arp.getSenderProtocolAddress());
194 + Set<PortAddresses> sourceAddresses = findPortsInSubnet(source);
195 + boolean matched = false;
196 + for (PortAddresses pa : sourceAddresses) {
197 + for (InterfaceIpAddress ia : pa.ipAddresses()) {
198 + if (ia.ipAddress().equals(source) &&
199 + pa.vlan().equals(vlan)) {
200 + matched = true;
201 + sendTo(eth, pa.connectPoint());
202 + break;
203 + }
204 + }
205 + }
206 +
207 + if (matched) {
210 return; 208 return;
211 } 209 }
212 210
213 // 211 //
214 - // Reply on the port the request was received on 212 + // The request couldn't be resolved.
213 + // Flood the request on all ports except the incoming port.
215 // 214 //
216 - Ethernet arpReply = buildArpReply(targetAddress, dst.mac(), eth); 215 + flood(eth, inPort);
217 - sendTo(arpReply, inPort); 216 + return;
218 } 217 }
219 218
220 private void replyNdp(Ethernet eth, ConnectPoint inPort) { 219 private void replyNdp(Ethernet eth, ConnectPoint inPort) {
......
...@@ -242,8 +242,8 @@ public class ProxyArpManagerTest { ...@@ -242,8 +242,8 @@ public class ProxyArpManagerTest {
242 } 242 }
243 243
244 /** 244 /**
245 - * Tests {@link ProxyArpManager#isKnown(Ip4Address)} in the case where the 245 + * Tests {@link ProxyArpManager#isKnown(org.onlab.packet.IpAddress)} in the
246 - * IP address is not known. 246 + * case where the IP address is not known.
247 * Verifies the method returns false. 247 * Verifies the method returns false.
248 */ 248 */
249 @Test 249 @Test
...@@ -255,8 +255,8 @@ public class ProxyArpManagerTest { ...@@ -255,8 +255,8 @@ public class ProxyArpManagerTest {
255 } 255 }
256 256
257 /** 257 /**
258 - * Tests {@link ProxyArpManager#isKnown(Ip4Address)} in the case where the 258 + * Tests {@link ProxyArpManager#isKnown(org.onlab.packet.IpAddress)} in the
259 - * IP address is known. 259 + * case where the IP address is known.
260 * Verifies the method returns true. 260 * Verifies the method returns true.
261 */ 261 */
262 @Test 262 @Test
...@@ -403,12 +403,14 @@ public class ProxyArpManagerTest { ...@@ -403,12 +403,14 @@ public class ProxyArpManagerTest {
403 403
404 @Test 404 @Test
405 public void testReplyToRequestFromUs() { 405 public void testReplyToRequestFromUs() {
406 - replay(hostService); // no further host service expectations
407 -
408 Ip4Address ourIp = Ip4Address.valueOf("10.0.1.1"); 406 Ip4Address ourIp = Ip4Address.valueOf("10.0.1.1");
409 MacAddress ourMac = MacAddress.valueOf(1L); 407 MacAddress ourMac = MacAddress.valueOf(1L);
410 Ip4Address theirIp = Ip4Address.valueOf("10.0.1.100"); 408 Ip4Address theirIp = Ip4Address.valueOf("10.0.1.100");
411 409
410 + expect(hostService.getHostsByIp(theirIp)).andReturn(Collections.emptySet());
411 + expect(hostService.getHost(HostId.hostId(ourMac, VLAN1))).andReturn(null);
412 + replay(hostService);
413 +
412 // This is a request from something inside our network (like a BGP 414 // This is a request from something inside our network (like a BGP
413 // daemon) to an external host. 415 // daemon) to an external host.
414 Ethernet arpRequest = buildArp(ARP.OP_REQUEST, ourMac, null, ourIp, theirIp); 416 Ethernet arpRequest = buildArp(ARP.OP_REQUEST, ourMac, null, ourIp, theirIp);
......