Skip to content

Commit

Permalink
Agent crash@FlowProto::FlowEventHandler
Browse files Browse the repository at this point in the history
Problem:
Interface was transitioning its oper ip forwarding state. Flow was being created
using same interface, however because of transition interface didnt add l3 nh to
flow_key of vmi. Flow requested for flow key which was NULL and crashed.

Solution:Create L3 nh based on l3 config in vmi.

Change-Id: I0d4a972ffb9238c6459729d0171c3d6b0e343a20
Closes-bug: #1612049
(cherry picked from commit 8e4e442)
  • Loading branch information
manishsing committed Aug 17, 2016
1 parent 98cf3ef commit 3b44ff2
Show file tree
Hide file tree
Showing 3 changed files with 70 additions and 35 deletions.
4 changes: 2 additions & 2 deletions src/vnsw/agent/oper/test/test_intf.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1655,8 +1655,8 @@ TEST_F(IntfTest, IntfActivateDeactivate_5) {
InterfaceNHKey bridge_policy_nh_key(intf_key5, true,
InterfaceNHFlags::BRIDGE, mac);
client->WaitForIdle();
EXPECT_FALSE(FindNH(&unicast_nh_key));
EXPECT_FALSE(FindNH(&unicast_policy_nh_key));
EXPECT_TRUE(FindNH(&unicast_nh_key));
EXPECT_TRUE(FindNH(&unicast_policy_nh_key));
EXPECT_TRUE(FindNH(&bridge_nh_key));
EXPECT_TRUE(FindNH(&bridge_policy_nh_key));

Expand Down
98 changes: 65 additions & 33 deletions src/vnsw/agent/oper/vm_interface.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1502,26 +1502,7 @@ bool VmInterface::Delete(const DBRequest *req) {
void VmInterface::UpdateL3MetadataIp(VrfEntry *old_vrf, bool force_update,
bool policy_change,
bool old_metadata_ip_active) {
InterfaceTable *table = static_cast<InterfaceTable *>(get_table());
Agent *agent = table->agent();
assert(metadata_ip_active_);
if (!old_metadata_ip_active) {
InterfaceNH::CreateL3VmInterfaceNH(GetUuid(),
vm_mac_,
vrf_->GetName());
InterfaceNHKey key1(new VmInterfaceKey(AgentKey::ADD_DEL_CHANGE,
GetUuid(), ""),
true, InterfaceNHFlags::INET4,
vm_mac_);
l3_interface_nh_policy_ = static_cast<NextHop *>(agent->
nexthop_table()->FindActiveEntry(&key1));
InterfaceNHKey key2(new VmInterfaceKey(AgentKey::ADD_DEL_CHANGE,
GetUuid(), ""),
false, InterfaceNHFlags::INET4,
vm_mac_);
l3_interface_nh_no_policy_ = static_cast<NextHop *>(agent->
nexthop_table()->FindActiveEntry(&key2));
}
UpdateL3TunnelId(force_update, policy_change);
UpdateMetadataRoute(old_metadata_ip_active, old_vrf);
}
Expand All @@ -1531,12 +1512,6 @@ void VmInterface::DeleteL3MetadataIp(VrfEntry *old_vrf, bool force_update,
bool old_metadata_ip_active,
bool old_need_linklocal_ip) {
assert(!metadata_ip_active_);
if (old_metadata_ip_active) {
InterfaceNH::DeleteL3InterfaceNH(GetUuid(),
vm_mac_);
l3_interface_nh_policy_.reset();
l3_interface_nh_no_policy_.reset();
}
DeleteL3TunnelId();
DeleteMetadataRoute(old_metadata_ip_active, old_vrf,
old_need_linklocal_ip);
Expand Down Expand Up @@ -1725,18 +1700,31 @@ void VmInterface::ApplyConfigCommon(const VrfEntry *old_vrf,
}

/*
* L2 nexthops:
* These L2 nexthop are used by multicast and bridge.
* Presence of multicast forces it to be present in
* ipv4 mode(l3-only).
*
* L3 nexthops:
* Also creates L3 interface NH, if layer3_forwarding is set.
* It does not depend on oper state of ip forwarding.
* Needed as health check can disable oper ip_active and will result in flow key
* pointing to L2 interface NH. This has to be avoided as interface still points
* to l3 nh and flow should use same. For this fix it is also required that l3
* i/f nh is also created on seeing config and not oper state of l3. Reason
* being if vmi(irrespective of health check) is coming up and transitioning
* from ipv4_inactive to ip4_active and during this transition a flow is added
* then flow_key in vmi will return null because l3 config is set and interface
* nh not created yet.
*/
void VmInterface::UpdateCommonNextHop() {
if (IsActive()) {
UpdateL2NextHop();
return;
} else {
DeleteL2NextHop();
return;
}
UpdateL2NextHop();
UpdateL3NextHop();
}

void VmInterface::DeleteCommonNextHop() {
DeleteL2NextHop();
DeleteL3NextHop();
}

void VmInterface::ApplyMacVmBindingConfig(const VrfEntry *old_vrf,
Expand Down Expand Up @@ -1799,7 +1787,9 @@ void VmInterface::ApplyConfig(bool old_ipv4_active, bool old_l2_active,
}

//Update common prameters
UpdateCommonNextHop();
if (IsActive()) {
UpdateCommonNextHop();
}
// Add/Update L3 Metadata
if (metadata_ip_active_) {
UpdateL3MetadataIp(old_vrf, force_update, policy_change,
Expand Down Expand Up @@ -1852,6 +1842,9 @@ void VmInterface::ApplyConfig(bool old_ipv4_active, bool old_l2_active,
// Remove Alias-ip entries marked for deletion
CleanupAliasIpList();

if (!IsActive()) {
DeleteCommonNextHop();
}
InterfaceTable *table = static_cast<InterfaceTable *>(get_table());
if (old_l2_active != l2_active_) {
if (l2_active_) {
Expand Down Expand Up @@ -3045,6 +3038,10 @@ bool VmInterface::Ipv6Deactivated(bool old_ipv6_active) {
}

void VmInterface::UpdateFlowKeyNextHop() {
if (!IsActive()) {
flow_key_nh_.reset();
return;
}
InterfaceTable *table = static_cast<InterfaceTable *>(get_table());
Agent *agent = table->agent();

Expand Down Expand Up @@ -3079,6 +3076,41 @@ void VmInterface::UpdateMacVmBinding() {
this);
}

void VmInterface::UpdateL3NextHop() {
InterfaceTable *table = static_cast<InterfaceTable *>(get_table());
Agent *agent = table->agent();
//layer3_forwarding config is not set, so delete as i/f is active.
if (!layer3_forwarding_) {
if ((l3_interface_nh_policy_.get() != NULL) ||
(l3_interface_nh_no_policy_.get() != NULL)) {
DeleteL3NextHop();
}
return;
}

InterfaceNH::CreateL3VmInterfaceNH(GetUuid(),
vm_mac_,
vrf_->GetName());
InterfaceNHKey key1(new VmInterfaceKey(AgentKey::ADD_DEL_CHANGE,
GetUuid(), ""),
true, InterfaceNHFlags::INET4,
vm_mac_);
l3_interface_nh_policy_ =
static_cast<NextHop *>(agent->nexthop_table()->FindActiveEntry(&key1));
InterfaceNHKey key2(new VmInterfaceKey(AgentKey::ADD_DEL_CHANGE,
GetUuid(), ""),
false, InterfaceNHFlags::INET4,
vm_mac_);
l3_interface_nh_no_policy_ =
static_cast<NextHop *>(agent->nexthop_table()->FindActiveEntry(&key2));
}

void VmInterface::DeleteL3NextHop() {
InterfaceNH::DeleteL3InterfaceNH(GetUuid(), vm_mac_);
l3_interface_nh_policy_.reset();
l3_interface_nh_no_policy_.reset();
}

//Create these NH irrespective of mode, as multicast uses l2 NH.
void VmInterface::UpdateL2NextHop() {
InterfaceTable *table = static_cast<InterfaceTable *>(get_table());
Expand Down
3 changes: 3 additions & 0 deletions src/vnsw/agent/oper/vm_interface.h
Original file line number Diff line number Diff line change
Expand Up @@ -797,6 +797,8 @@ class VmInterface : public Interface {
void UpdateL3TunnelId(bool force_update, bool policy_change);
void DeleteL3TunnelId();
void UpdateMacVmBinding();
void UpdateL3NextHop();
void DeleteL3NextHop();
void UpdateL2NextHop();
void UpdateFlowKeyNextHop();
void DeleteL2NextHop();
Expand Down Expand Up @@ -871,6 +873,7 @@ class VmInterface : public Interface {
bool UpdateIsHealthCheckActive();
void CopyEcmpLoadBalance(EcmpLoadBalance &ecmp_load_balance);
void UpdateCommonNextHop();
void DeleteCommonNextHop();

VmEntryBackRef vm_;
VnEntryRef vn_;
Expand Down

0 comments on commit 3b44ff2

Please sign in to comment.