From 01e4f46c4f3d8132868fa0405ed0b63860ed0963 Mon Sep 17 00:00:00 2001 From: kiranvasudeva Date: Mon, 2 Apr 2018 20:39:10 +0530 Subject: [PATCH] NETVIRT-1181: 3 issues related to port-udpate with Allowed address pairs are addressed in this fix. 1. AAP added as port-udpate, is added for L2Broadcast allow rule. 2. AAP deleted as port-update, is handled to remove L2Broadcast allow rule only if there is no other aap with same mac. 3. AAP deleted as port-udpate, is handled to remove Dhcp v4 & v6 allow rule only if there is no other aap with same mac Change-Id: Id422f2dd828265d12da4cc03b41f6cd4e6967569 Signed-off-by: kiranvasudeva --- .../aclservice/EgressAclServiceImpl.java | 57 ++++++++++++++----- 1 file changed, 42 insertions(+), 15 deletions(-) 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 c9b8951f25..299ce0794f 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 @@ -124,12 +124,12 @@ public class EgressAclServiceImpl extends AbstractAclServiceImpl { egressAclDhcpv6DropServerTraffic(dpid, lportTag, addOrRemove); egressAclIcmpv6DropRouterAdvts(dpid, lportTag, addOrRemove); egressAclIcmpv6AllowedList(dpid, lportTag, addOrRemove); - programL2BroadcastAllowRule(port, addOrRemove); } List filteredAAPs = AclServiceUtils.excludeMulticastAAPs(allowedAddresses); + programL2BroadcastAllowRule(port, filteredAAPs, addOrRemove); - egressAclDhcpAllowClientTraffic(dpid, filteredAAPs, lportTag, addOrRemove); - egressAclDhcpv6AllowClientTraffic(dpid, filteredAAPs, lportTag, addOrRemove); + egressAclDhcpAllowClientTraffic(port, filteredAAPs, lportTag, addOrRemove); + egressAclDhcpv6AllowClientTraffic(port, filteredAAPs, lportTag, addOrRemove); programArpRule(dpid, filteredAAPs, lportTag, addOrRemove); } @@ -237,14 +237,18 @@ public class EgressAclServiceImpl extends AbstractAclServiceImpl { /** * Add rule to ensure only DHCP server traffic from the specified mac is allowed. - * - * @param dpId the dpid + * @param port the Acl Interface port * @param allowedAddresses the allowed addresses * @param lportTag the lport tag * @param addOrRemove whether to add or remove the flow */ - private void egressAclDhcpAllowClientTraffic(BigInteger dpId, List allowedAddresses, + private void egressAclDhcpAllowClientTraffic(AclInterface port, List allowedAddresses, int lportTag, int addOrRemove) { + // if there is a duplicate mac with different aap, do not delete the Dhcp Allow rule. + if (hasDuplicateMac(port.getAllowedAddressPairs(), allowedAddresses)) { + return; + } + BigInteger dpId = port.getDpId(); List instructions = getDispatcherTableResubmitInstructions(); for (AllowedAddressPairs aap : allowedAddresses) { if (!AclServiceUtils.isIPv4Address(aap)) { @@ -265,14 +269,18 @@ public class EgressAclServiceImpl extends AbstractAclServiceImpl { /** * Add rule to ensure only DHCPv6 server traffic from the specified mac is * allowed. - * - * @param dpId the dpid + * @param port the Acl Interface port * @param allowedAddresses the allowed addresses * @param lportTag the lport tag * @param addOrRemove whether to add or remove the flow */ - private void egressAclDhcpv6AllowClientTraffic(BigInteger dpId, List allowedAddresses, + private void egressAclDhcpv6AllowClientTraffic(AclInterface port, List allowedAddresses, int lportTag, int addOrRemove) { + // if there is a duplicate mac with different aap, do not delete the Dhcp Allow rule. + if (hasDuplicateMac(port.getAllowedAddressPairs(), allowedAddresses)) { + return; + } + BigInteger dpId = port.getDpId(); List instructions = getDispatcherTableResubmitInstructions(); for (AllowedAddressPairs aap : allowedAddresses) { if (AclServiceUtils.isIPv4Address(aap)) { @@ -333,21 +341,25 @@ public class EgressAclServiceImpl extends AbstractAclServiceImpl { */ @Override protected void programBroadcastRules(AclInterface port, int addOrRemove) { - programL2BroadcastAllowRule(port, addOrRemove); + programL2BroadcastAllowRule(port, AclServiceUtils.excludeMulticastAAPs(port.getAllowedAddressPairs()), + addOrRemove); } /** * Programs Non-IP broadcast rules. - * * @param port the Acl Interface port + * @param filteredAAPs the filtered AAPs list * @param addOrRemove whether to delete or add flow */ - private void programL2BroadcastAllowRule(AclInterface port, int addOrRemove) { + private void programL2BroadcastAllowRule(AclInterface port, List filteredAAPs, + int addOrRemove) { + // if there is a duplicate mac with different aap, do not delete the Broadcast rule. + if (hasDuplicateMac(port.getAllowedAddressPairs(), filteredAAPs)) { + return; + } BigInteger dpId = port.getDpId(); int lportTag = port.getLPortTag(); - List allowedAddresses = port.getAllowedAddressPairs(); - Set macs = - allowedAddresses.stream().map(AllowedAddressPairs::getMacAddress).collect(Collectors.toSet()); + Set macs = filteredAAPs.stream().map(aap -> aap.getMacAddress()).collect(Collectors.toSet()); for (MacAddress mac : macs) { List matches = new ArrayList<>(); matches.add(new MatchEthernetSource(mac)); @@ -361,6 +373,21 @@ public class EgressAclServiceImpl extends AbstractAclServiceImpl { } } + private boolean hasDuplicateMac(List allowedAddresses, + List filteredAAPs) { + // Do not proceed further if VM delete or Port down event. + if (allowedAddresses.size() == filteredAAPs.size()) { + return false; + } + //exclude filteredAAP entries from port's AAP's before comparison + List filteredAllowedAddressed = allowedAddresses.stream().filter( + aap -> !filteredAAPs.contains(aap)).collect(Collectors.toList()); + Set macs = filteredAAPs.stream().map(aap -> aap.getMacAddress()).collect(Collectors.toSet()); + List aapWithDuplicateMac = filteredAllowedAddressed.stream() + .filter(entry -> macs.contains(entry.getMacAddress())).collect(Collectors.toList()); + return !aapWithDuplicateMac.isEmpty(); + } + @Override protected boolean isValidDirection(Class direction) { return direction.equals(DirectionEgress.class); -- 2.36.6