From 6e211a47c07e3e608caf53673b7e04be38e4c6e0 Mon Sep 17 00:00:00 2001 From: Ning Zhang Date: Tue, 12 Nov 2019 17:10:43 +0800 Subject: [PATCH] Optimize issues in acl flow table class 1.Delete method programAcl() in AbstractAclServiceImpl.java 2.String concatenation does not require the toString method Change-Id: I3579c52e95688a1fd02c3c3fbb491b2809d318be Signed-off-by: Ning Zhang --- .../aclservice/AbstractAclServiceImpl.java | 19 ++++--- .../aclservice/EgressAclServiceImpl.java | 6 +-- .../aclservice/IngressAclServiceImpl.java | 50 ++++++++++--------- 3 files changed, 38 insertions(+), 37 deletions(-) diff --git a/aclservice/impl/src/main/java/org/opendaylight/netvirt/aclservice/AbstractAclServiceImpl.java b/aclservice/impl/src/main/java/org/opendaylight/netvirt/aclservice/AbstractAclServiceImpl.java index 65645cbcf7..dda7afd35d 100644 --- a/aclservice/impl/src/main/java/org/opendaylight/netvirt/aclservice/AbstractAclServiceImpl.java +++ b/aclservice/impl/src/main/java/org/opendaylight/netvirt/aclservice/AbstractAclServiceImpl.java @@ -119,8 +119,9 @@ public abstract class AbstractAclServiceImpl implements AclServiceListener { return false; } BigInteger dpId = port.getDpId(); + String portId = port.getInterfaceId(); if (dpId == null || port.getLPortTag() == null) { - LOG.error("Unable to find DpId from ACL interface with id {}", port.getInterfaceId()); + LOG.error("Unable to find DpId from ACL interface with id {}", portId); return false; } @@ -130,13 +131,14 @@ public abstract class AbstractAclServiceImpl implements AclServiceListener { programDhcpService(flowEntries, port, Action.ADD, NwConstants.ADD_FLOW); } else { if (port.getSecurityGroups() == null) { - LOG.info("Port {} without SGs", port.getInterfaceId()); + LOG.info("Port {} without SGs", portId); return false; } - programAcl(flowEntries, port, Action.ADD, NwConstants.ADD_FLOW); + programAclWithAllowedAddress(flowEntries, port, port.getAllowedAddressPairs(), + Action.ADD, NwConstants.ADD_FLOW); updateRemoteAclFilterTable(flowEntries, port, NwConstants.ADD_FLOW); } - programFlows(AclConstants.ACL_JOB_KEY_PREFIX + port.getInterfaceId(), flowEntries, NwConstants.ADD_FLOW); + programFlows(AclConstants.ACL_JOB_KEY_PREFIX + portId, flowEntries, NwConstants.ADD_FLOW); return true; } @@ -343,10 +345,6 @@ public abstract class AbstractAclServiceImpl implements AclServiceListener { matches, instructions, addOrRemove); } - private void programAcl(List flowEntries, AclInterface port, Action action, int addOrRemove) { - programAclWithAllowedAddress(flowEntries, port, port.getAllowedAddressPairs(), action, addOrRemove); - } - private void programAclWithAllowedAddress(List flowEntries, AclInterface port, List allowedAddresses, Action action, int addOrRemove) { Uint64 dpId = Uint64.valueOf(port.getDpId()); @@ -532,7 +530,8 @@ public abstract class AbstractAclServiceImpl implements AclServiceListener { if (port.getInterfaceType() == InterfaceType.DhcpService) { programDhcpService(flowEntries, port, Action.REMOVE, NwConstants.DEL_FLOW); } else { - programAcl(flowEntries, port, Action.REMOVE, NwConstants.DEL_FLOW); + programAclWithAllowedAddress(flowEntries, port, port.getAllowedAddressPairs(), + Action.REMOVE, NwConstants.DEL_FLOW); updateRemoteAclFilterTable(flowEntries, port, NwConstants.DEL_FLOW); } programFlows(AclConstants.ACL_JOB_KEY_PREFIX + port.getInterfaceId(), flowEntries, NwConstants.DEL_FLOW); @@ -979,7 +978,7 @@ public abstract class AbstractAclServiceImpl implements AclServiceListener { } String flowName = - this.directionString + "_Fixed_Conntrk_" + dpId.toString() + "_" + this.directionString + "_Fixed_Conntrk_" + dpId + "_" + lportTag + "_" + matchEtherType + "_Recirc"; addFlowEntryToList(flowEntries, dpId, getAclConntrackSenderTable(), flowName, AclConstants.ACL_DEFAULT_PRIORITY, 0, 0, AclConstants.COOKIE_ACL_BASE, matches, instructions, diff --git a/aclservice/impl/src/main/java/org/opendaylight/netvirt/aclservice/EgressAclServiceImpl.java b/aclservice/impl/src/main/java/org/opendaylight/netvirt/aclservice/EgressAclServiceImpl.java index 7c82042c94..3ac7e2d37b 100644 --- a/aclservice/impl/src/main/java/org/opendaylight/netvirt/aclservice/EgressAclServiceImpl.java +++ b/aclservice/impl/src/main/java/org/opendaylight/netvirt/aclservice/EgressAclServiceImpl.java @@ -182,7 +182,7 @@ public class EgressAclServiceImpl extends AbstractAclServiceImpl { | MetaDataUtil.METADATA_MASK_ACL_DROP.longValue()); matches.add(new MatchMetadata(metaData, metaDataMask)); - String flowName = "Egress_" + dpId.toString() + "_" + lportTag + "_Drop"; + String flowName = "Egress_" + dpId + "_" + lportTag + "_Drop"; addFlowEntryToList(flowEntries, dpId, getAclCommitterTable(), flowName, AclConstants.CT_STATE_TRACKED_INVALID_PRIORITY, 0, 0, AclServiceUtils.getDropFlowCookie(lportTag), matches, instructions, addOrRemove); @@ -218,7 +218,7 @@ public class EgressAclServiceImpl extends AbstractAclServiceImpl { List gotoInstructions = new ArrayList<>(); gotoInstructions.add(new InstructionGotoTable(getAclConntrackClassifierTable())); - String flowName = "Egress_Fixed_Goto_Classifier_" + dpId.toString() + "_" + lportTag + "_" + String flowName = "Egress_Fixed_Goto_Classifier_" + dpId + "_" + lportTag + "_" + mac.getValue() + "_" + attachIp.stringValue(); addFlowEntryToList(flowEntries, dpId, getAclAntiSpoofingTable(), flowName, AclConstants.PROTO_MATCH_PRIORITY, 0, 0, AclConstants.COOKIE_ACL_BASE, matches, gotoInstructions, @@ -240,7 +240,7 @@ public class EgressAclServiceImpl extends AbstractAclServiceImpl { for (Integer icmpv6Type: AclConstants.allowedIcmpv6NdList()) { List matches = AclServiceUtils.buildIcmpV6Matches(icmpv6Type, 0, lportTag, serviceMode); - String flowName = "Egress_ICMPv6" + "_" + dpId.toString() + "_" + lportTag + "_" + icmpv6Type + "_Permit_"; + String flowName = "Egress_ICMPv6" + "_" + dpId + "_" + lportTag + "_" + icmpv6Type + "_Permit_"; addFlowEntryToList(flowEntries, dpId, getAclAntiSpoofingTable(), flowName, AclConstants.PROTO_IPV6_ALLOWED_PRIORITY, 0, 0, AclConstants.COOKIE_ACL_BASE, matches, instructions, addOrRemove); diff --git a/aclservice/impl/src/main/java/org/opendaylight/netvirt/aclservice/IngressAclServiceImpl.java b/aclservice/impl/src/main/java/org/opendaylight/netvirt/aclservice/IngressAclServiceImpl.java index 7964e6ec72..9b0c3a00b1 100644 --- a/aclservice/impl/src/main/java/org/opendaylight/netvirt/aclservice/IngressAclServiceImpl.java +++ b/aclservice/impl/src/main/java/org/opendaylight/netvirt/aclservice/IngressAclServiceImpl.java @@ -108,8 +108,7 @@ public class IngressAclServiceImpl extends AbstractAclServiceImpl { AclServiceUtils.getBoundServices(String.format("%s.%s.%s", "acl", "egressacl", interfaceName), serviceIndex, flowPriority, AclConstants.COOKIE_ACL_BASE, instructions); InstanceIdentifier path = AclServiceUtils.buildServiceId(interfaceName, - ServiceIndex.getIndex(NwConstants.EGRESS_ACL_SERVICE_NAME, NwConstants.EGRESS_ACL_SERVICE_INDEX), - serviceMode); + serviceIndex, serviceMode); return Collections.singletonList( txRunner.callWithNewWriteOnlyTransactionAndSubmit(CONFIGURATION, tx -> tx.put( @@ -206,7 +205,7 @@ public class IngressAclServiceImpl extends AbstractAclServiceImpl { MetaDataUtil.getLportTagMaskForReg6())); matches.add(new MatchMetadata(metaData, metaDataMask)); - String flowName = "Ingress_" + dpId.toString() + "_" + lportTag + "_Drop"; + String flowName = "Ingress_" + dpId + "_" + lportTag + "_Drop"; addFlowEntryToList(flowEntries, dpId, getAclCommitterTable(), flowName, AclConstants.CT_STATE_TRACKED_INVALID_PRIORITY, 0, 0, AclServiceUtils.getDropFlowCookie(lportTag), matches, instructions, addOrRemove); @@ -227,7 +226,7 @@ public class IngressAclServiceImpl extends AbstractAclServiceImpl { List gotoInstructions = new ArrayList<>(); gotoInstructions.add(new InstructionGotoTable(getAclConntrackClassifierTable())); - String flowName = "Ingress_Fixed_Goto_Classifier_" + dpId.toString() + "_" + lportTag + "_" + String flowName = "Ingress_Fixed_Goto_Classifier_" + dpId + "_" + lportTag + "_" + mac.getValue() + "_" + attachIp.stringValue(); addFlowEntryToList(flowEntries, dpId, getAclAntiSpoofingTable(), flowName, AclConstants.PROTO_MATCH_PRIORITY, 0, 0, AclConstants.COOKIE_ACL_BASE, matches, gotoInstructions, @@ -282,7 +281,7 @@ public class IngressAclServiceImpl extends AbstractAclServiceImpl { AclConstants.DHCP_CLIENT_PORT_IPV6, lportTag, serviceMode); List instructions = getDispatcherTableResubmitInstructions(); - String flowName = "Ingress_DHCP_Server_v6" + "_" + dpId.toString() + "_" + lportTag + "_Permit_"; + String flowName = "Ingress_DHCP_Server_v6" + "_" + dpId + "_" + lportTag + "_Permit_"; addFlowEntryToList(flowEntries, dpId, getAclAntiSpoofingTable(), flowName, AclConstants.PROTO_DHCP_SERVER_MATCH_PRIORITY, 0, 0, AclConstants.COOKIE_ACL_BASE, matches, instructions, addOrRemove); @@ -335,24 +334,27 @@ public class IngressAclServiceImpl extends AbstractAclServiceImpl { @Override protected void programIcmpv6RARule(List flowEntries, AclInterface port, List subnets, int addOrRemove) { - if (AclServiceUtils.isIpv6Subnet(subnets)) { - /* Allow ICMPv6 Router Advertisement packets from external routers as well as internal routers - * if subnet is configured with IPv6 version - * Allow ICMPv6 Router Advertisement packets if originating from any LinkLocal Address. - */ - List instructions = getDispatcherTableResubmitInstructions(); - List matches = - AclServiceUtils.buildIcmpV6Matches(AclConstants.ICMPV6_TYPE_RA, 0, - port.getLPortTag(), serviceMode); - matches.addAll(AclServiceUtils.buildIpMatches( - new IpPrefixOrAddress(IpPrefixBuilder.getDefaultInstance(AclConstants.IPV6_LINK_LOCAL_PREFIX)), - AclServiceManager.MatchCriteria.MATCH_SOURCE)); - String flowName = "Ingress_ICMPv6" + "_" + port.getDpId() + "_" + port.getLPortTag() + "_" - + AclConstants.ICMPV6_TYPE_RA + "_LinkLocal_Permit_"; - addFlowEntryToList(flowEntries, Uint64.valueOf(port.getDpId()), getAclAntiSpoofingTable(), flowName, - AclConstants.PROTO_IPV6_ALLOWED_PRIORITY, 0, 0, AclConstants.COOKIE_ACL_BASE, matches, - instructions, addOrRemove); + if (!AclServiceUtils.isIpv6Subnet(subnets)) { + return; } + + Uint64 dpid = Uint64.valueOf(port.getDpId()); + /* Allow ICMPv6 Router Advertisement packets from external routers as well as internal routers + * if subnet is configured with IPv6 version + * Allow ICMPv6 Router Advertisement packets if originating from any LinkLocal Address. + */ + List instructions = getDispatcherTableResubmitInstructions(); + List matches = + AclServiceUtils.buildIcmpV6Matches(AclConstants.ICMPV6_TYPE_RA, 0, + port.getLPortTag(), serviceMode); + matches.addAll(AclServiceUtils.buildIpMatches( + new IpPrefixOrAddress(IpPrefixBuilder.getDefaultInstance(AclConstants.IPV6_LINK_LOCAL_PREFIX)), + AclServiceManager.MatchCriteria.MATCH_SOURCE)); + String flowName = "Ingress_ICMPv6" + "_" + dpid + "_" + port.getLPortTag() + "_" + + AclConstants.ICMPV6_TYPE_RA + "_LinkLocal_Permit_"; + addFlowEntryToList(flowEntries, dpid, getAclAntiSpoofingTable(), flowName, + AclConstants.PROTO_IPV6_ALLOWED_PRIORITY, 0, 0, AclConstants.COOKIE_ACL_BASE, matches, + instructions, addOrRemove); } /** @@ -424,7 +426,7 @@ public class IngressAclServiceImpl extends AbstractAclServiceImpl { matches.add(lportMatchInfo); List instructions = new ArrayList<>(); instructions.add(new InstructionGotoTable(getAclConntrackClassifierTable())); - String flowName = "Ingress_v4_Broadcast_" + dpId.longValue() + "_" + String flowName = "Ingress_v4_Broadcast_" + dpId + "_" + lportTag + "_" + broadcastAddress + "_Permit"; addFlowEntryToList(flowEntries, dpId, getAclAntiSpoofingTable(), flowName, AclConstants.PROTO_MATCH_PRIORITY, 0, 0, AclConstants.COOKIE_ACL_BASE, matches, instructions, @@ -516,7 +518,7 @@ public class IngressAclServiceImpl extends AbstractAclServiceImpl { matches.add(icmpTypeMatchInfo); List instructions = getDispatcherTableResubmitInstructions(); - String flowName = "Ingress_DHCP_Service_ICMP_" + dpId.toString() + "_" + lportTag + "_" + icmpType + "_Permit_"; + String flowName = "Ingress_DHCP_Service_ICMP_" + dpId + "_" + lportTag + "_" + icmpType + "_Permit_"; addFlowEntryToList(flowEntries, dpId, getAclAntiSpoofingTable(), flowName, AclConstants.PROTO_DHCP_SERVER_MATCH_PRIORITY, 0, 0, AclConstants.COOKIE_ACL_BASE, matches, instructions, addOrRemove); -- 2.36.6