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 <zhangninglc@inspur.com>

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 65645cbcf789c46903eafcd8d62ae68d178dddde..dda7afd35db5f728e4c0dfccbd377484c10f09ec 100644 (file)
--- 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<FlowEntity> flowEntries, AclInterface port, Action action, int addOrRemove) {
-        programAclWithAllowedAddress(flowEntries, port, port.getAllowedAddressPairs(), action, addOrRemove);
-    }
-
     private void programAclWithAllowedAddress(List<FlowEntity> flowEntries, AclInterface port,
             List<AllowedAddressPairs> 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 7c82042c94858c83940bf4a8f0399a1965d5cef0..3ac7e2d37b338989339adfc7f3f53fabece5eab7 100644 (file)
--- 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<InstructionInfo> 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<MatchInfoBase> 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 7964e6ec720a8ede052f090107724268c3f7fd72..9b0c3a00b1f7cf45a3ad5fdfdcbfdebef8a5b0c4 100644 (file)
--- 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<BoundServices> 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<InstructionInfo> 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<InstructionInfo> 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<FlowEntity> flowEntries, AclInterface port, List<SubnetInfo> 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<InstructionInfo> instructions = getDispatcherTableResubmitInstructions();
-            List<MatchInfoBase> 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<InstructionInfo> instructions = getDispatcherTableResubmitInstructions();
+        List<MatchInfoBase> 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<InstructionInfo> 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<InstructionInfo> 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);