Skip to content

Commit

Permalink
If I install a software simple gateway (vgw) on a compute
Browse files Browse the repository at this point in the history
node and create in one virtual network 2 virtual machines,
each of them with default security group and attach a
floating IP to each of those 2 VMs I can ping by default
the VM which runs on the compute node where the vgw was
installed but cannot ping the VM which is runing on the
second compute node.
The normal behavior should be that by default (as long
as in the security default rule the ingress rule uses
the default security group as "Address" instead of
0.0.0.0/0 the ping on floating IPs should not work.
Code needs to be added to treat the special case of the
vgw interface - which is an interface of type INET and
sub-type SIMPLE_GATEWAY. After these changes the security
group rules will be respected for floating IPs on both
compute nodes.

Cherry-Pick from review:
https://review.opencontrail.org/#/c/38460/

Change-Id: If05f3b61471a95f6b123be7f86ff2cdbb9d011eb
Partial-Bug: #1736972
  • Loading branch information
esnagendra committed Jul 23, 2018
1 parent 6895c5c commit 9cec5dd
Showing 1 changed file with 39 additions and 14 deletions.
53 changes: 39 additions & 14 deletions src/vnsw/agent/pkt/flow_entry.cc
Expand Up @@ -1555,7 +1555,14 @@ void FlowEntry::GetPolicyInfo(const VnEntry *vn, const FlowEntry *rflow) {
if (data_.intf_entry == NULL)
return;

if (data_.intf_entry->type() != Interface::VM_INTERFACE)
bool vgw_pass = true;
if (data_.intf_entry->type() == Interface::INET) {
vgw_pass = false;
InetInterface* inet_intf = (InetInterface*)(data_.intf_entry).get();
if ((inet_intf != NULL) && (inet_intf->sub_type() == InetInterface::SIMPLE_GATEWAY))
vgw_pass = true;
}
if ((data_.intf_entry->type() != Interface::VM_INTERFACE) && !vgw_pass)
return;

// Get Network policy/mirror cfg policy/mirror policies
Expand Down Expand Up @@ -1700,13 +1707,21 @@ void FlowEntry::GetSgList(const Interface *intf) {

// Get virtual-machine port for forward flow
const VmInterface *vm_port = NULL;
bool vgw_pass = false;
if (intf != NULL) {
if (intf->type() == Interface::VM_INTERFACE) {
if (intf->type() == Interface::INET) {
const InetInterface* inet_intf = static_cast<const InetInterface *>(intf);
if ((inet_intf != NULL) && (inet_intf->sub_type() == InetInterface::SIMPLE_GATEWAY)) {
vgw_pass = true;
}
}
if (intf->type() == Interface::VM_INTERFACE) {
vm_port = static_cast<const VmInterface *>(intf);
}
vgw_pass = true;
}
}

if (vm_port == NULL) {
if (!vgw_pass) {
return;
}

Expand Down Expand Up @@ -1834,8 +1849,10 @@ static bool CopySgEntries(const VmInterface *vm_port, bool ingress_acl,
void FlowEntry::GetLocalFlowSgList(const VmInterface *vm_port,
const VmInterface *reverse_vm_port) {
// Get SG-Rule for the forward flow
data_.match_p.sg_policy.rule_present = CopySgEntries(vm_port, true,
data_.match_p.sg_policy.m_acl_l);
if (vm_port) {
data_.match_p.sg_policy.rule_present =
CopySgEntries(vm_port, true, data_.match_p.sg_policy.m_acl_l);
}
// For local flow, we need to simulate SG lookup at both ends.
// Assume packet is from VM-A to VM-B.
// If we apply Ingress-ACL from VM-A, then apply Egress-ACL from VM-B
Expand All @@ -1854,9 +1871,11 @@ void FlowEntry::GetLocalFlowSgList(const VmInterface *vm_port,
// the flow if either forward or reverse flow is allowed

// Copy the SG rules to be applied for reverse flow
data_.match_p.sg_policy.reverse_out_rule_present =
CopySgEntries(vm_port, false,
data_.match_p.sg_policy.m_reverse_out_acl_l);
if (vm_port) {
data_.match_p.sg_policy.reverse_out_rule_present =
CopySgEntries(vm_port, false,
data_.match_p.sg_policy.m_reverse_out_acl_l);
}

if (reverse_vm_port) {
data_.match_p.sg_policy.reverse_rule_present =
Expand All @@ -1868,8 +1887,12 @@ void FlowEntry::GetLocalFlowSgList(const VmInterface *vm_port,
void FlowEntry::GetNonLocalFlowSgList(const VmInterface *vm_port) {
// Get SG-Rule for the forward flow
bool ingress = is_flags_set(FlowEntry::IngressDir);
data_.match_p.sg_policy.rule_present = CopySgEntries(vm_port, ingress,
data_.match_p.sg_policy.m_acl_l);
if (vm_port) {
data_.match_p.sg_policy.rule_present =
CopySgEntries(vm_port, ingress,
data_.match_p.sg_policy.m_acl_l);
}

data_.match_p.sg_policy.out_rule_present = false;

//Update reverse SG flow entry so that it can be used in below 2 scenario
Expand All @@ -1881,9 +1904,11 @@ void FlowEntry::GetNonLocalFlowSgList(const VmInterface *vm_port) {
// the flow if either forward or reverse flow is allowed

// Copy the SG rules to be applied for reverse flow
data_.match_p.sg_policy.reverse_out_rule_present =
CopySgEntries(vm_port, !ingress,
data_.match_p.sg_policy.m_reverse_out_acl_l);
if (vm_port) {
data_.match_p.sg_policy.reverse_out_rule_present =
CopySgEntries(vm_port, !ingress,
data_.match_p.sg_policy.m_reverse_out_acl_l);
}
data_.match_p.sg_policy.reverse_rule_present = false;
}

Expand Down

0 comments on commit 9cec5dd

Please sign in to comment.