Fix: start commit the conntrack only if the flow in the whitelist
1. Tag VNI and VID for ICMP reply packet initiated from exGW
2. Do not remove ICMP reply match rules when reset reactive SNAT
rules
3. Fix incorrect SNAT IP retrieval methods for external gateway
Change-Id: I9649161e9661636ea93f04d71159949d9281f4ae
diff --git a/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/OpenstackRoutingArpHandler.java b/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/OpenstackRoutingArpHandler.java
index 304258f..6ae7100 100644
--- a/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/OpenstackRoutingArpHandler.java
+++ b/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/OpenstackRoutingArpHandler.java
@@ -76,6 +76,7 @@
import org.openstack4j.model.network.Network;
import org.openstack4j.model.network.Port;
import org.openstack4j.model.network.Router;
+import org.openstack4j.model.network.RouterInterface;
import org.osgi.service.component.ComponentContext;
import org.slf4j.Logger;
@@ -515,27 +516,33 @@
}
private void setFakeGatewayArpRuleByRouter(Router router, boolean install) {
- setFakeGatewayArpRuleByGateway(router.getExternalGatewayInfo(), install);
+ setFakeGatewayArpRuleByGateway(router.getId(), router.getExternalGatewayInfo(), install);
}
- private Set<IP> getExternalGatewaySnatIps(ExternalGateway extGw) {
- return osNetworkAdminService.ports().stream()
- .filter(port ->
- Objects.equals(port.getNetworkId(), extGw.getNetworkId()))
- .filter(port ->
- Objects.equals(port.getDeviceOwner(), DEVICE_OWNER_ROUTER_GW))
- .flatMap(port -> port.getFixedIps().stream())
+ private Set<IP> getExternalGatewaySnatIps(String routerId, ExternalGateway extGw) {
+ if (routerId == null) {
+ return ImmutableSet.of();
+ }
+
+ Set<String> portIds = osRouterAdminService.routerInterfaces(routerId).stream()
+ .map(RouterInterface::getPortId)
+ .collect(Collectors.toSet());
+
+ return portIds.stream()
+ .map(pid -> osNetworkAdminService.port(pid))
+ .filter(p -> Objects.equals(p.getDeviceOwner(), DEVICE_OWNER_ROUTER_GW))
+ .flatMap(p -> p.getFixedIps().stream())
.collect(Collectors.toSet());
}
- private void setFakeGatewayArpRuleByGateway(ExternalGateway extGw, boolean install) {
+ private void setFakeGatewayArpRuleByGateway(String routerId, ExternalGateway extGw, boolean install) {
if (ARP_BROADCAST_MODE.equals(getArpMode())) {
if (extGw == null) {
return;
}
- setFakeGatewayArpRuleByIps(getExternalGatewaySnatIps(extGw), install);
+ setFakeGatewayArpRuleByIps(getExternalGatewaySnatIps(routerId, extGw), install);
}
}
@@ -649,13 +656,15 @@
case OPENSTACK_ROUTER_GATEWAY_ADDED:
eventExecutor.execute(() ->
// add a gateway manually after adding a router
- setFakeGatewayArpRuleByGateway(event.externalGateway(), true)
+ setFakeGatewayArpRuleByGateway(event.subject().getId(),
+ event.externalGateway(), true)
);
break;
case OPENSTACK_ROUTER_GATEWAY_REMOVED:
eventExecutor.execute(() ->
// remove a gateway from an existing router
- setFakeGatewayArpRuleByGateway(event.externalGateway(), false)
+ setFakeGatewayArpRuleByGateway(event.subject().getId(),
+ event.externalGateway(), false)
);
break;
case OPENSTACK_FLOATING_IP_ASSOCIATED:
diff --git a/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/OpenstackRoutingHandler.java b/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/OpenstackRoutingHandler.java
index a831216..89b50df 100644
--- a/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/OpenstackRoutingHandler.java
+++ b/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/OpenstackRoutingHandler.java
@@ -850,7 +850,6 @@
.matchIPSrc(srcSubnet)
.matchEthDst(Constants.DEFAULT_GATEWAY_MAC);
-
switch (networkType) {
case VXLAN:
sBuilder.matchTunnelId(Long.parseLong(segmentId));
@@ -882,7 +881,14 @@
GW_COMMON_TABLE,
install);
+ // TODO: we do not remove the IcmpReplyMatchRules with false installation flag
+ // need to find a better way to remove this rule
+ if (install) {
+ setIcmpReplyRules(deviceId, install);
+ }
+ }
+ private void setIcmpReplyRules(DeviceId deviceId, boolean install) {
// Sends ICMP response to controller for SNATing ingress traffic
TrafficSelector selector = DefaultTrafficSelector.builder()
.matchEthType(Ethernet.TYPE_IPV4)
diff --git a/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/OpenstackRoutingIcmpHandler.java b/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/OpenstackRoutingIcmpHandler.java
index d1ab9d7..9197082 100644
--- a/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/OpenstackRoutingIcmpHandler.java
+++ b/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/OpenstackRoutingIcmpHandler.java
@@ -93,6 +93,9 @@
private static final String ERR_REQ = "Failed to handle ICMP request: ";
private static final String ERR_DUPLICATE = " already exists";
+ private static final String VXLAN = "VXLAN";
+ private static final String VLAN = "VLAN";
+
@Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
protected CoreService coreService;
@@ -407,13 +410,26 @@
}
private void sendReply(Ethernet icmpReply, InstancePort instPort) {
- TrafficTreatment treatment = DefaultTrafficTreatment.builder()
- .setOutput(instPort.portNumber())
- .build();
+ TrafficTreatment.Builder tBuilder = DefaultTrafficTreatment.builder()
+ .setOutput(instPort.portNumber());
+
+ String netId = instPort.networkId();
+ String segId = osNetworkService.segmentId(netId);
+
+ switch (osNetworkService.networkType(netId)) {
+ case VXLAN:
+ tBuilder.setTunnelId(Long.valueOf(segId));
+ break;
+ case VLAN:
+ tBuilder.setVlanId(VlanId.vlanId(segId));
+ break;
+ default:
+ break;
+ }
OutboundPacket packet = new DefaultOutboundPacket(
instPort.deviceId(),
- treatment,
+ tBuilder.build(),
ByteBuffer.wrap(icmpReply.serialize()));
packetService.emit(packet);
@@ -435,7 +451,8 @@
return;
}
- if (!gateways.isEmpty() && !gateways.contains(context.inPacket().receivedFrom().deviceId())) {
+ if (!gateways.isEmpty() &&
+ !gateways.contains(context.inPacket().receivedFrom().deviceId())) {
return;
}
diff --git a/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/OpenstackSecurityGroupHandler.java b/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/OpenstackSecurityGroupHandler.java
index 672ce8b..7fe5fce 100644
--- a/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/OpenstackSecurityGroupHandler.java
+++ b/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/OpenstackSecurityGroupHandler.java
@@ -49,6 +49,7 @@
import org.onosproject.net.flow.TrafficSelector;
import org.onosproject.net.flow.TrafficTreatment;
import org.onosproject.net.flow.criteria.ExtensionSelector;
+import org.onosproject.net.flow.instructions.ExtensionTreatment;
import org.onosproject.openstacknetworking.api.InstancePort;
import org.onosproject.openstacknetworking.api.InstancePortAdminService;
import org.onosproject.openstacknetworking.api.InstancePortEvent;
@@ -172,13 +173,13 @@
.build();
private final InstancePortListener instancePortListener =
- new InternalInstancePortListener();
+ new InternalInstancePortListener();
private final OpenstackNetworkListener osNetworkListener =
- new InternalOpenstackNetworkListener();
+ new InternalOpenstackNetworkListener();
private final OpenstackNetworkListener osPortListener =
- new InternalOpenstackPortListener();
+ new InternalOpenstackPortListener();
private final OpenstackSecurityGroupListener securityGroupListener =
- new InternalSecurityGroupListener();
+ new InternalSecurityGroupListener();
private final OpenstackNodeListener osNodeListener = new InternalNodeListener();
private ConsistentMap<String, Port> removedOsPortStore;
@@ -331,10 +332,10 @@
SecurityGroupRule rSgRule =
new NeutronSecurityGroupRule
.SecurityGroupRuleConcreteBuilder()
- .from(sgRule)
- .direction(sgRule.getDirection().toUpperCase()
- .equals(EGRESS) ? INGRESS : EGRESS)
- .build();
+ .from(sgRule)
+ .direction(sgRule.getDirection().toUpperCase()
+ .equals(EGRESS) ? INGRESS : EGRESS)
+ .build();
populateSecurityGroupRule(rSgRule, instPort, port,
rInstPort.ipAddress().toIpPrefix(), install);
populateSecurityGroupRule(rSgRule, rInstPort, port,
@@ -343,7 +344,7 @@
} else {
populateSecurityGroupRule(sgRule, instPort, port,
sgRule.getRemoteIpPrefix() == null ? IP_PREFIX_ANY :
- IpPrefix.valueOf(sgRule.getRemoteIpPrefix()), install);
+ IpPrefix.valueOf(sgRule.getRemoteIpPrefix()), install);
}
}
@@ -358,14 +359,25 @@
return;
}
+ // XXX All egress traffic needs to go through connection tracking module,
+ // which might hurt its performance.
+ ExtensionTreatment ctTreatment =
+ niciraConnTrackTreatmentBuilder(driverService, instPort.deviceId())
+ .commit(true)
+ .build();
+
+ TrafficTreatment treatment = DefaultTrafficTreatment.builder()
+ .extension(ctTreatment, instPort.deviceId())
+ .transition(JUMP_TABLE)
+ .build();
+
selectors.forEach(selector ->
osFlowRuleService.setRule(appId,
- instPort.deviceId(),
- selector,
- DefaultTrafficTreatment.builder().transition(JUMP_TABLE).build(),
- PRIORITY_ACL_RULE,
- ACL_TABLE,
- install));
+ instPort.deviceId(),
+ selector, treatment,
+ PRIORITY_ACL_RULE,
+ ACL_TABLE,
+ install));
}
/**
@@ -486,7 +498,7 @@
sgRule.getPortRangeMin() < sgRule.getPortRangeMax()) {
Map<TpPort, TpPort> portRangeMatchMap =
buildPortRangeMatches(sgRule.getPortRangeMin(),
- sgRule.getPortRangeMax());
+ sgRule.getPortRangeMax());
portRangeMatchMap.forEach((key, value) -> {
if (sgRule.getProtocol().toUpperCase().equals(PROTO_TCP)) {
@@ -523,9 +535,6 @@
sgRule.getPortRangeMin() == null ? 0 : sgRule.getPortRangeMin(),
sgRule.getPortRangeMax() == null ? 0 : sgRule.getPortRangeMax());
buildMatchRemoteIp(sBuilder, remoteIp, sgRule.getDirection());
- if (sgRule.getRemoteGroupId() != null && sgRule.getRemoteGroupId().isEmpty()) {
- buildMatchRemoteIp(sBuilder, remoteIp, sgRule.getDirection());
- }
}
private void buildTunnelId(TrafficSelector.Builder sBuilder, Port port) {
@@ -536,6 +545,8 @@
sBuilder.matchVlanId(VlanId.vlanId(segId));
} else if (VXLAN.equals(netType)) {
sBuilder.matchTunnelId(Long.valueOf(segId));
+ } else {
+ log.warn("Cannot tag the VID due to lack of support of virtual network type {}", netType);
}
}
@@ -628,13 +639,13 @@
}
log.info("Reset security group info " +
- (useSecurityGroup ? " with " : " without") + " Security Group");
+ (useSecurityGroup ? " with " : " without") + " Security Group");
}
private void securityGroupRuleAdded(SecurityGroupRule sgRule) {
osNetService.ports().stream()
.filter(port -> port.getSecurityGroups()
- .contains(sgRule.getSecurityGroupId()))
+ .contains(sgRule.getSecurityGroupId()))
.forEach(port -> {
updateSecurityGroupRule(
instancePortService.instancePort(port.getId()),
@@ -649,7 +660,7 @@
Sets.union(osNetService.ports(), removedPorts).stream()
.filter(port -> port.getSecurityGroups()
- .contains(sgRule.getSecurityGroupId()))
+ .contains(sgRule.getSecurityGroupId()))
.forEach(port -> {
updateSecurityGroupRule(
instancePortService.instancePort(port.getId()),
@@ -819,7 +830,7 @@
}
private class InternalOpenstackNetworkListener
- implements OpenstackNetworkListener {
+ implements OpenstackNetworkListener {
@Override
public boolean isRelevant(OpenstackNetworkEvent event) {
@@ -875,7 +886,7 @@
}
private class InternalSecurityGroupListener
- implements OpenstackSecurityGroupListener {
+ implements OpenstackSecurityGroupListener {
@Override
public boolean isRelevant(OpenstackSecurityGroupEvent event) {
diff --git a/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/OpenstackSwitchingHandler.java b/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/OpenstackSwitchingHandler.java
index 1d74e9a..e63444f 100644
--- a/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/OpenstackSwitchingHandler.java
+++ b/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/OpenstackSwitchingHandler.java
@@ -39,7 +39,6 @@
import org.onosproject.net.flow.DefaultTrafficTreatment;
import org.onosproject.net.flow.TrafficSelector;
import org.onosproject.net.flow.TrafficTreatment;
-import org.onosproject.net.flow.instructions.ExtensionTreatment;
import org.onosproject.openstacknetworking.api.InstancePort;
import org.onosproject.openstacknetworking.api.InstancePortEvent;
import org.onosproject.openstacknetworking.api.InstancePortListener;
@@ -49,7 +48,6 @@
import org.onosproject.openstacknetworking.api.OpenstackNetworkListener;
import org.onosproject.openstacknetworking.api.OpenstackNetworkService;
import org.onosproject.openstacknetworking.api.OpenstackSecurityGroupService;
-import org.onosproject.openstacknetworking.util.RulePopulatorUtil;
import org.onosproject.openstacknode.api.OpenstackNode;
import org.onosproject.openstacknode.api.OpenstackNodeService;
import org.openstack4j.model.network.Network;
@@ -500,25 +498,21 @@
.matchInPort(instPort.portNumber())
.build();
- // XXX All egress traffic needs to go through connection tracking module,
- // which might hurt its performance.
- ExtensionTreatment ctTreatment =
- RulePopulatorUtil.niciraConnTrackTreatmentBuilder(driverService, instPort.deviceId())
- .commit(true).build();
+ TrafficTreatment.Builder tBuilder = DefaultTrafficTreatment.builder()
+ .setTunnelId(getVni(instPort));
- TrafficTreatment.Builder tb = DefaultTrafficTreatment.builder()
- .setTunnelId(getVni(instPort))
- .transition(ARP_TABLE);
- if (securityGroupService.isSecurityGroupEnabled() && ethType == Ethernet.TYPE_IPV4) {
- tb.extension(ctTreatment, instPort.deviceId());
+ if (ethType == Ethernet.TYPE_ARP) {
+ tBuilder.transition(ARP_TABLE);
+ } else if (ethType == Ethernet.TYPE_IPV4) {
+ tBuilder.transition(ACL_TABLE);
}
osFlowRuleService.setRule(
appId,
instPort.deviceId(),
selector,
- tb.build(),
+ tBuilder.build(),
PRIORITY_TUNNEL_TAG_RULE,
VTAG_TABLE,
install);