Below ACL issues are fixed:
(a) Issue: Tenant VM IP address failures
Fix: Cache was not updated properly in AclInterfaceStateListener. This was
resulting in wrong lport and dpnid in cache. Updated to rectify this
problem in AclInterfaceStateListener.
(b) Issue: Ids exhausted for pool : ACL-TAG-POOL
Fix: In Event listener there was wrong check for SecurityRule in
add(), remove() and update() methods. The check in remove() some times
was resulting in releaseId of ACL-TAG-POOL not being called. This was
resulting in Ids exhausted when large number of ACLs were created and
deleted. Check for SecurityRule in add(), remove() and update() is
removed to fix the problem.
(b) Issue: ACL-TAG-POOL error with releaseId
Fix: In the current code, releaseId of ACL-TAG-POOL is called on all
the nodes of cluster. On one node releaseId would have succeeded and
on other two nodes, error reported would be observed. Code is updated
now to invoke releaseId with Entirty check so that it gets executed on
only one node instead of all the nodes.
Change-Id: Iba0e6486795cfd867db03487b24c069ac06f35eb
Signed-off-by: Shashidhar Raja <shashidharr@altencalsoftlabs.com>
protected void programAceRule(List<FlowEntity> flowEntries, AclInterface port, String aclName, Ace ace,
int addOrRemove) {
SecurityRuleAttr aceAttr = AclServiceUtils.getAccessListAttributes(ace);
protected void programAceRule(List<FlowEntity> flowEntries, AclInterface port, String aclName, Ace ace,
int addOrRemove) {
SecurityRuleAttr aceAttr = AclServiceUtils.getAccessListAttributes(ace);
+ if (aceAttr == null) {
+ LOG.error("Ace {} of Acl {} is either null or not having SecurityRuleAttr",
+ ((ace == null) ? null : ace.getRuleName()), aclName);
+ return;
+ }
if (addOrRemove == NwConstants.ADD_FLOW && aceAttr.isDeleted()) {
LOG.trace("Ignoring {} rule which is already deleted", ace.getRuleName());
return;
if (addOrRemove == NwConstants.ADD_FLOW && aceAttr.isDeleted()) {
LOG.trace("Ignoring {} rule which is already deleted", ace.getRuleName());
return;
@Override
protected void remove(InstanceIdentifier<Acl> key, Acl acl) {
@Override
protected void remove(InstanceIdentifier<Acl> key, Acl acl) {
- String aclName = acl.getAclName();
- if (!AclServiceUtils.isOfAclInterest(acl)) {
- LOG.trace("{} does not have SecurityRuleAttr augmentation", aclName);
- return;
- }
-
LOG.trace("On remove event, remove ACL: {}", acl);
LOG.trace("On remove event, remove ACL: {}", acl);
+ String aclName = acl.getAclName();
this.aclDataUtil.removeAcl(aclName);
Integer aclTag = this.aclDataUtil.getAclTag(aclName);
if (aclTag != null) {
this.aclDataUtil.removeAclTag(aclName);
this.aclDataUtil.removeAcl(aclName);
Integer aclTag = this.aclDataUtil.getAclTag(aclName);
if (aclTag != null) {
this.aclDataUtil.removeAclTag(aclName);
- this.aclServiceUtils.releaseAclTag(aclName);
}
updateRemoteAclCache(acl.getAccessListEntries().getAce(), aclName, AclServiceManager.Action.REMOVE);
if (aclClusterUtil.isEntityOwner()) {
}
updateRemoteAclCache(acl.getAccessListEntries().getAce(), aclName, AclServiceManager.Action.REMOVE);
if (aclClusterUtil.isEntityOwner()) {
+ if (aclTag != null) {
+ this.aclServiceUtils.releaseAclTag(aclName);
+ }
// Handle Rule deletion If SG Remove event is received before SG Rule delete event
List<Ace> aceList = acl.getAccessListEntries().getAce();
Collection<AclInterface> aclInterfaces =
// Handle Rule deletion If SG Remove event is received before SG Rule delete event
List<Ace> aceList = acl.getAccessListEntries().getAce();
Collection<AclInterface> aclInterfaces =
@Override
protected void update(InstanceIdentifier<Acl> key, Acl aclBefore, Acl aclAfter) {
@Override
protected void update(InstanceIdentifier<Acl> key, Acl aclBefore, Acl aclAfter) {
- if (!AclServiceUtils.isOfAclInterest(aclAfter) && !AclServiceUtils.isOfAclInterest(aclBefore)) {
- LOG.trace("before {} and after {} does not have SecurityRuleAttr augmentation",
- aclBefore.getAclName(), aclAfter.getAclName());
- return;
- }
String aclName = aclAfter.getAclName();
Collection<AclInterface> interfacesBefore =
ImmutableSet.copyOf(aclDataUtil.getInterfaceList(new Uuid(aclName)));
String aclName = aclAfter.getAclName();
Collection<AclInterface> interfacesBefore =
ImmutableSet.copyOf(aclDataUtil.getInterfaceList(new Uuid(aclName)));
@Override
protected void add(InstanceIdentifier<Acl> key, Acl acl) {
@Override
protected void add(InstanceIdentifier<Acl> key, Acl acl) {
- String aclName = acl.getAclName();
- if (!AclServiceUtils.isOfAclInterest(acl)) {
- LOG.trace("{} does not have SecurityRuleAttr augmentation", aclName);
- return;
- }
-
LOG.trace("On add event, add ACL: {}", acl);
this.aclDataUtil.addAcl(acl);
LOG.trace("On add event, add ACL: {}", acl);
this.aclDataUtil.addAcl(acl);
+ String aclName = acl.getAclName();
Integer aclTag = this.aclServiceUtils.allocateAclTag(aclName);
if (aclTag != null && aclTag != AclConstants.INVALID_ACL_TAG) {
this.aclDataUtil.addAclTag(aclName, aclTag);
Integer aclTag = this.aclServiceUtils.allocateAclTag(aclName);
if (aclTag != null && aclTag != AclConstants.INVALID_ACL_TAG) {
this.aclDataUtil.addAclTag(aclName, aclTag);
if (AclServiceUtils.isOfInterest(aclInterface)) {
List<Uuid> aclList = aclInterface.getSecurityGroups();
if (aclList != null) {
if (AclServiceUtils.isOfInterest(aclInterface)) {
List<Uuid> aclList = aclInterface.getSecurityGroups();
if (aclList != null) {
- aclDataUtil.addAclInterfaceMap(aclList, aclInterface);
+ aclDataUtil.addOrUpdateAclInterfaceMap(aclList, aclInterface);
}
if (aclInterface.getElanId() == null) {
LOG.debug("On Add event, skip ADD since ElanId is not updated");
}
if (aclInterface.getElanId() == null) {
LOG.debug("On Add event, skip ADD since ElanId is not updated");
return this.aclMap.get(aclName);
}
return this.aclMap.get(aclName);
}
- public void addAclInterfaceMap(List<Uuid> aclList, AclInterface port) {
- for (Uuid acl : aclList) {
- addAclInterface(acl, port);
- }
- }
-
- private void addAclInterface(Uuid acl, AclInterface port) {
- aclInterfaceMap.computeIfAbsent(acl, key -> new ConcurrentHashMap<>())
- .putIfAbsent(port.getInterfaceId(), port);
- }
-
public void addOrUpdateAclInterfaceMap(List<Uuid> aclList, AclInterface port) {
for (Uuid acl : aclList) {
aclInterfaceMap.computeIfAbsent(acl, key -> new ConcurrentHashMap<>()).put(port.getInterfaceId(), port);
public void addOrUpdateAclInterfaceMap(List<Uuid> aclList, AclInterface port) {
for (Uuid acl : aclList) {
aclInterfaceMap.computeIfAbsent(acl, key -> new ConcurrentHashMap<>()).put(port.getInterfaceId(), port);
@Nullable
public static SecurityRuleAttr getAccessListAttributes(Ace ace) {
if (ace == null) {
@Nullable
public static SecurityRuleAttr getAccessListAttributes(Ace ace) {
if (ace == null) {
- LOG.error("Ace is Null");
return null;
}
SecurityRuleAttr aceAttributes = ace.augmentation(SecurityRuleAttr.class);
if (aceAttributes == null) {
return null;
}
SecurityRuleAttr aceAttributes = ace.augmentation(SecurityRuleAttr.class);
if (aceAttributes == null) {
- LOG.error("Ace is null");
return null;
}
return aceAttributes;
return null;
}
return aceAttributes;
- public static boolean isOfAclInterest(Acl acl) {
- if (acl.getAccessListEntries() != null) {
- List<Ace> aceList = acl.getAccessListEntries().getAce();
- if (aceList != null && !aceList.isEmpty()) {
- return aceList.get(0).augmentation(SecurityRuleAttr.class) != null;
- }
- }
- return false;
- }
-
/**
* Builds the ip protocol matches.
*
/**
* Builds the ip protocol matches.
*
if (accessListEntries != null && accessListEntries.getAce() != null) {
for (Ace ace : accessListEntries.getAce()) {
SecurityRuleAttr aceAttr = AclServiceUtils.getAccessListAttributes(ace);
if (accessListEntries != null && accessListEntries.getAce() != null) {
for (Ace ace : accessListEntries.getAce()) {
SecurityRuleAttr aceAttr = AclServiceUtils.getAccessListAttributes(ace);
- if (Objects.equals(aceAttr.getDirection(), direction) && doesAceHaveRemoteGroupId(aceAttr)) {
+ if (aceAttr != null && Objects.equals(aceAttr.getDirection(), direction)
+ && doesAceHaveRemoteGroupId(aceAttr)) {
remoteAclIds.add(aceAttr.getRemoteGroupId());
}
}
remoteAclIds.add(aceAttr.getRemoteGroupId());
}
}
final BigInteger dpId = new BigInteger("123");
assertFalse(aclDataUtil.doesDpnHaveAclInterface(dpId));
final BigInteger dpId = new BigInteger("123");
assertFalse(aclDataUtil.doesDpnHaveAclInterface(dpId));
- aclDataUtil.addAclInterfaceMap(Arrays.asList(ACL1, ACL2), PORT1);
+ aclDataUtil.addOrUpdateAclInterfaceMap(Arrays.asList(ACL1, ACL2), PORT1);
assertAclInterfaces(ACL1, PORT1);
assertAclInterfaces(ACL2, PORT1);
assertAclInterfaces(ACL1, PORT1);
assertAclInterfaces(ACL2, PORT1);
- aclDataUtil.addAclInterfaceMap(Arrays.asList(ACL1), PORT2);
+ aclDataUtil.addOrUpdateAclInterfaceMap(Arrays.asList(ACL1), PORT2);
assertAclInterfaces(ACL1, PORT1, PORT2);
assertAclInterfaces(ACL2, PORT1);
assertFalse(aclDataUtil.doesDpnHaveAclInterface(dpId));
assertAclInterfaces(ACL1, PORT1, PORT2);
assertAclInterfaces(ACL2, PORT1);
assertFalse(aclDataUtil.doesDpnHaveAclInterface(dpId));
- aclDataUtil.addAclInterfaceMap(Arrays.asList(ACL1), PORT2);
+ aclDataUtil.addOrUpdateAclInterfaceMap(Arrays.asList(ACL1), PORT2);
assertAclInterfaces(ACL1, PORT1, PORT2);
aclDataUtil.addOrUpdateAclInterfaceMap(Arrays.asList(ACL1), PORT3);
assertAclInterfaces(ACL1, PORT1, PORT2);
aclDataUtil.addOrUpdateAclInterfaceMap(Arrays.asList(ACL1), PORT3);
assertNotNull(map);
assertEquals(0, map.size());
assertNotNull(map);
assertEquals(0, map.size());
- aclDataUtil.addAclInterfaceMap(Arrays.asList(ACL2), PORT1);
- aclDataUtil.addAclInterfaceMap(Arrays.asList(ACL2), PORT2);
+ aclDataUtil.addOrUpdateAclInterfaceMap(Arrays.asList(ACL2), PORT1);
+ aclDataUtil.addOrUpdateAclInterfaceMap(Arrays.asList(ACL2), PORT2);
map = aclDataUtil.getRemoteAclInterfaces(ACL1, direction);
assertEquals(1, map.size());
assertAclInterfaces(map.get(ACL2.getValue()), PORT1, PORT2);
map = aclDataUtil.getRemoteAclInterfaces(ACL1, direction);
assertEquals(1, map.size());
assertAclInterfaces(map.get(ACL2.getValue()), PORT1, PORT2);
- aclDataUtil.addAclInterfaceMap(Arrays.asList(ACL3), PORT3);
+ aclDataUtil.addOrUpdateAclInterfaceMap(Arrays.asList(ACL3), PORT3);
map = aclDataUtil.getRemoteAclInterfaces(ACL1, direction);
assertEquals(2, map.size());
assertAclInterfaces(map.get(ACL2.getValue()), PORT1, PORT2);
map = aclDataUtil.getRemoteAclInterfaces(ACL1, direction);
assertEquals(2, map.size());
assertAclInterfaces(map.get(ACL2.getValue()), PORT1, PORT2);