Hyunsun Moon
Committed by Gerrit Code Review

ONOS-3549 Fixed NPE during renew for rangeNotEnforced IP

Added renew case for IP assigned with rangeNotEnforced option.
Addresses ONOS-3549

Change-Id: I6cb43f662332f0d461889d32659f1252ea436102
...@@ -25,11 +25,13 @@ import org.apache.felix.scr.annotations.Service; ...@@ -25,11 +25,13 @@ import org.apache.felix.scr.annotations.Service;
25 import org.onlab.packet.Ip4Address; 25 import org.onlab.packet.Ip4Address;
26 import org.onlab.packet.MacAddress; 26 import org.onlab.packet.MacAddress;
27 import org.onlab.util.KryoNamespace; 27 import org.onlab.util.KryoNamespace;
28 +import org.onlab.util.Tools;
28 import org.onosproject.dhcp.DhcpStore; 29 import org.onosproject.dhcp.DhcpStore;
29 import org.onosproject.dhcp.IpAssignment; 30 import org.onosproject.dhcp.IpAssignment;
30 import org.onosproject.net.HostId; 31 import org.onosproject.net.HostId;
31 import org.onosproject.store.serializers.KryoNamespaces; 32 import org.onosproject.store.serializers.KryoNamespaces;
32 import org.onosproject.store.service.ConsistentMap; 33 import org.onosproject.store.service.ConsistentMap;
34 +import org.onosproject.store.service.ConsistentMapException;
33 import org.onosproject.store.service.DistributedSet; 35 import org.onosproject.store.service.DistributedSet;
34 import org.onosproject.store.service.Serializer; 36 import org.onosproject.store.service.Serializer;
35 import org.onosproject.store.service.StorageService; 37 import org.onosproject.store.service.StorageService;
...@@ -42,6 +44,7 @@ import java.util.Map; ...@@ -42,6 +44,7 @@ import java.util.Map;
42 import java.util.List; 44 import java.util.List;
43 import java.util.HashMap; 45 import java.util.HashMap;
44 import java.util.Objects; 46 import java.util.Objects;
47 +import java.util.concurrent.atomic.AtomicBoolean;
45 48
46 /** 49 /**
47 * Manages the pool of available IP Addresses in the network and 50 * Manages the pool of available IP Addresses in the network and
...@@ -68,6 +71,8 @@ public class DistributedDhcpStore implements DhcpStore { ...@@ -68,6 +71,8 @@ public class DistributedDhcpStore implements DhcpStore {
68 // Hardcoded values are default values. 71 // Hardcoded values are default values.
69 72
70 private static int timeoutForPendingAssignments = 60; 73 private static int timeoutForPendingAssignments = 60;
74 + private static final int MAX_RETRIES = 3;
75 + private static final int MAX_BACKOFF = 10;
71 76
72 @Activate 77 @Activate
73 protected void activate() { 78 protected void activate() {
...@@ -165,72 +170,85 @@ public class DistributedDhcpStore implements DhcpStore { ...@@ -165,72 +170,85 @@ public class DistributedDhcpStore implements DhcpStore {
165 @Override 170 @Override
166 public boolean assignIP(HostId hostId, Ip4Address ipAddr, int leaseTime, boolean rangeNotEnforced, 171 public boolean assignIP(HostId hostId, Ip4Address ipAddr, int leaseTime, boolean rangeNotEnforced,
167 List<Ip4Address> addressList) { 172 List<Ip4Address> addressList) {
168 -
169 - IpAssignment assignmentInfo;
170 -
171 log.debug("Assign IP Called w/ Ip4Address: {}, HostId: {}", ipAddr.toString(), hostId.mac().toString()); 173 log.debug("Assign IP Called w/ Ip4Address: {}, HostId: {}", ipAddr.toString(), hostId.mac().toString());
172 174
173 - if (allocationMap.containsKey(hostId)) { 175 + AtomicBoolean assigned = Tools.retryable(() -> {
174 - 176 + AtomicBoolean result = new AtomicBoolean(false);
175 - assignmentInfo = allocationMap.get(hostId).value(); 177 + allocationMap.compute(
176 - IpAssignment.AssignmentStatus status = assignmentInfo.assignmentStatus(); 178 + hostId,
177 - 179 + (h, existingAssignment) -> {
178 - if (Objects.equals(assignmentInfo.ipAddress(), ipAddr) && ipWithinRange(ipAddr)) { 180 + IpAssignment assignment = existingAssignment;
179 - 181 + if (existingAssignment == null) {
180 - if (status == IpAssignment.AssignmentStatus.Option_Assigned || 182 + if (rangeNotEnforced) {
181 - status == IpAssignment.AssignmentStatus.Option_Requested) { 183 + assignment = IpAssignment.builder()
182 - // Client has a currently active binding with the server. 184 + .ipAddress(ipAddr)
183 - assignmentInfo = IpAssignment.builder() 185 + .timestamp(new Date())
184 - .ipAddress(ipAddr) 186 + .leasePeriod(leaseTime)
185 - .timestamp(new Date()) 187 + .rangeNotEnforced(true)
186 - .leasePeriod(leaseTime) 188 + .assignmentStatus(IpAssignment.AssignmentStatus.Option_RangeNotEnforced)
187 - .assignmentStatus(IpAssignment.AssignmentStatus.Option_Assigned) 189 + .subnetMask((Ip4Address) addressList.toArray()[0])
188 - .build(); 190 + .dhcpServer((Ip4Address) addressList.toArray()[1])
189 - allocationMap.put(hostId, assignmentInfo); 191 + .routerAddress((Ip4Address) addressList.toArray()[2])
190 - return true; 192 + .domainServer((Ip4Address) addressList.toArray()[3])
191 - } else if (status == IpAssignment.AssignmentStatus.Option_Expired) { 193 + .build();
192 - // Client has an expired binding with the server. 194 + result.set(true);
193 - if (freeIPPool.contains(ipAddr)) { 195 + } else if (freeIPPool.remove(ipAddr)) {
194 - assignmentInfo = IpAssignment.builder() 196 + assignment = IpAssignment.builder()
195 - .ipAddress(ipAddr) 197 + .ipAddress(ipAddr)
196 - .timestamp(new Date()) 198 + .timestamp(new Date())
197 - .leasePeriod(leaseTime) 199 + .leasePeriod(leaseTime)
198 - .assignmentStatus(IpAssignment.AssignmentStatus.Option_Assigned) 200 + .assignmentStatus(IpAssignment.AssignmentStatus.Option_Assigned)
199 - .build(); 201 + .build();
200 - if (freeIPPool.remove(ipAddr)) { 202 + result.set(true);
201 - allocationMap.put(hostId, assignmentInfo); 203 + }
202 - return true; 204 + } else if (Objects.equals(existingAssignment.ipAddress(), ipAddr) &&
205 + (existingAssignment.rangeNotEnforced() || ipWithinRange(ipAddr))) {
206 + switch (existingAssignment.assignmentStatus()) {
207 + case Option_RangeNotEnforced:
208 + assignment = IpAssignment.builder()
209 + .ipAddress(ipAddr)
210 + .timestamp(new Date())
211 + .leasePeriod(leaseTime)
212 + .rangeNotEnforced(true)
213 + .assignmentStatus(IpAssignment.AssignmentStatus.Option_RangeNotEnforced)
214 + .subnetMask(existingAssignment.subnetMask())
215 + .dhcpServer(existingAssignment.dhcpServer())
216 + .routerAddress(existingAssignment.routerAddress())
217 + .domainServer(existingAssignment.domainServer())
218 + .build();
219 + result.set(true);
220 + break;
221 + case Option_Assigned:
222 + case Option_Requested:
223 + assignment = IpAssignment.builder()
224 + .ipAddress(ipAddr)
225 + .timestamp(new Date())
226 + .leasePeriod(leaseTime)
227 + .assignmentStatus(IpAssignment.AssignmentStatus.Option_Assigned)
228 + .build();
229 + result.set(true);
230 + break;
231 + case Option_Expired:
232 + if (freeIPPool.remove(ipAddr)) {
233 + assignment = IpAssignment.builder()
234 + .ipAddress(ipAddr)
235 + .timestamp(new Date())
236 + .leasePeriod(leaseTime)
237 + .assignmentStatus(IpAssignment.AssignmentStatus.Option_Assigned)
238 + .build();
239 + result.set(true);
240 + }
241 + break;
242 + default:
243 + break;
244 + }
203 } 245 }
204 - } 246 + return assignment;
205 - } 247 + });
206 - } 248 + return result;
207 - } else if (freeIPPool.contains(ipAddr)) { 249 + }, ConsistentMapException.class, MAX_RETRIES, MAX_BACKOFF).get();
208 - assignmentInfo = IpAssignment.builder() 250 +
209 - .ipAddress(ipAddr) 251 + return assigned.get();
210 - .timestamp(new Date())
211 - .leasePeriod(leaseTime)
212 - .assignmentStatus(IpAssignment.AssignmentStatus.Option_Assigned)
213 - .build();
214 - if (freeIPPool.remove(ipAddr)) {
215 - allocationMap.put(hostId, assignmentInfo);
216 - return true;
217 - }
218 - } else if (rangeNotEnforced) {
219 - assignmentInfo = IpAssignment.builder()
220 - .ipAddress(ipAddr)
221 - .timestamp(new Date())
222 - .leasePeriod(leaseTime)
223 - .rangeNotEnforced(true)
224 - .assignmentStatus(IpAssignment.AssignmentStatus.Option_RangeNotEnforced)
225 - .subnetMask((Ip4Address) addressList.toArray()[0])
226 - .dhcpServer((Ip4Address) addressList.toArray()[1])
227 - .routerAddress((Ip4Address) addressList.toArray()[2])
228 - .domainServer((Ip4Address) addressList.toArray()[3])
229 - .build();
230 - allocationMap.put(hostId, assignmentInfo);
231 - return true;
232 - }
233 - return false;
234 } 252 }
235 253
236 @Override 254 @Override
......