From 29629e8d01e9aa98f91ca330227c7e516c4a0c81 Mon Sep 17 00:00:00 2001 From: Ravi BK Date: Tue, 19 Jul 2016 20:25:04 +0530 Subject: [PATCH] AapMode change for existing AAP to be honoured. RootCause: If Allowed-Address Pair Mode is changed from active-standby to active-active or vice-versa, change is notified to Agent operDB, but Agent is not handling the change if AAP is already installed. Only change in ip-address/prefix-len of AAP is processed if it is already installed. Fix: Process the change and modify the route to ECMP when AAP mode is changed to active-active from default. Change-Id: I5d32ff5878fb51957e074417c907a0677c59e9d8 Closes-Bug: #1591886 --- src/vnsw/agent/oper/test/test_aap.cc | 30 +++++++++++++++++++++++ src/vnsw/agent/oper/vm_interface.cc | 36 ++++++++++++++++++---------- src/vnsw/agent/oper/vm_interface.h | 3 ++- 3 files changed, 55 insertions(+), 14 deletions(-) diff --git a/src/vnsw/agent/oper/test/test_aap.cc b/src/vnsw/agent/oper/test/test_aap.cc index 0f30aa60c14..4a42791ca62 100644 --- a/src/vnsw/agent/oper/test/test_aap.cc +++ b/src/vnsw/agent/oper/test/test_aap.cc @@ -48,6 +48,9 @@ void RouterIdDepInit(Agent *agent) { struct PortInfo input[] = { {"intf1", 1, "1.1.1.1", "00:00:00:01:01:01", 1, 1, "fd10::2"}, }; +IpamInfo ipam_info[] = { + {"1.1.1.0", 24, "1.1.1.10"}, +}; class TestAap : public ::testing::Test { public: @@ -143,6 +146,8 @@ class TestAap : public ::testing::Test { CreateVmportEnv(input, 1); client->WaitForIdle(); EXPECT_TRUE(VmPortActive(1)); + AddIPAM("vn1", ipam_info, 1); + client->WaitForIdle(); } virtual void TearDown() { @@ -151,6 +156,8 @@ class TestAap : public ::testing::Test { EXPECT_FALSE(VmPortFindRetDel(1)); EXPECT_FALSE(VrfFind("vrf1", true)); client->WaitForIdle(); + DelIPAM("vn1"); + client->WaitForIdle(); } protected: Peer *peer_; @@ -1237,6 +1244,29 @@ TEST_F(TestAap, StateMachine_20) { EXPECT_TRUE(path->path_preference().wait_for_traffic() == false); } +//Change Aap mode from default to active-active and verify ecmp route exists +TEST_F(TestAap, AapModeChange) { + Ip4Address aap_ip = Ip4Address::from_string("10.10.10.10"); + AddAap("intf1", 1, aap_ip, zero_mac.ToString()); + + VmInterface *vm_intf = VmInterfaceGet(1); + InetUnicastRouteEntry *rt = RouteGet("vrf1", aap_ip, 32); + const AgentPath *path = rt->FindPath(vm_intf->peer()); + EXPECT_TRUE(path->path_preference().sequence() == 0); + EXPECT_TRUE(path->path_preference().preference() == PathPreference::LOW); + EXPECT_TRUE(path->path_preference().ecmp() == false); + EXPECT_TRUE(path->path_preference().wait_for_traffic() == true); + + AddEcmpAap("intf1", 1, aap_ip); + EXPECT_TRUE(RouteFind("vrf1", aap_ip, 32)); + rt = RouteGet("vrf1", aap_ip, 32); + path = rt->FindPath(vm_intf->peer()); + EXPECT_TRUE(path->path_preference().sequence() == 0); + EXPECT_TRUE(path->path_preference().preference() == PathPreference::LOW); + EXPECT_TRUE(path->path_preference().ecmp() == true); + EXPECT_TRUE(path->path_preference().wait_for_traffic() == true); +} + int main(int argc, char *argv[]) { GETUSERARGS(); client = TestInit(init_file, ksync_init); diff --git a/src/vnsw/agent/oper/vm_interface.cc b/src/vnsw/agent/oper/vm_interface.cc index dd219dc12e2..6bf12e4a054 100644 --- a/src/vnsw/agent/oper/vm_interface.cc +++ b/src/vnsw/agent/oper/vm_interface.cc @@ -4071,18 +4071,20 @@ void VmInterface::StaticRouteList::Remove(StaticRouteSet::iterator &it) { /////////////////////////////////////////////////////////////////////////////// VmInterface::AllowedAddressPair::AllowedAddressPair() : ListEntry(), vrf_(""), addr_(), plen_(0), ecmp_(false), mac_(), - l2_entry_installed_(false), ethernet_tag_(0), vrf_ref_(NULL, this), - service_ip_(), label_(MplsTable::kInvalidLabel), policy_enabled_nh_(NULL), - policy_disabled_nh_(NULL) { + l2_entry_installed_(false), ecmp_config_changed_(false), ethernet_tag_(0), + vrf_ref_(NULL, this), service_ip_(), label_(MplsTable::kInvalidLabel), + policy_enabled_nh_(NULL), policy_disabled_nh_(NULL) { } VmInterface::AllowedAddressPair::AllowedAddressPair( const AllowedAddressPair &rhs) : ListEntry(rhs.installed_, rhs.del_pending_), vrf_(rhs.vrf_), addr_(rhs.addr_), plen_(rhs.plen_), ecmp_(rhs.ecmp_), mac_(rhs.mac_), - l2_entry_installed_(rhs.l2_entry_installed_), ethernet_tag_(rhs.ethernet_tag_), - vrf_ref_(rhs.vrf_ref_, this), service_ip_(rhs.service_ip_), - label_(rhs.label_), policy_enabled_nh_(rhs.policy_enabled_nh_), + l2_entry_installed_(rhs.l2_entry_installed_), + ecmp_config_changed_(rhs.ecmp_config_changed_), + ethernet_tag_(rhs.ethernet_tag_), vrf_ref_(rhs.vrf_ref_, this), + service_ip_(rhs.service_ip_), label_(rhs.label_), + policy_enabled_nh_(rhs.policy_enabled_nh_), policy_disabled_nh_(rhs.policy_disabled_nh_) { } @@ -4091,9 +4093,9 @@ VmInterface::AllowedAddressPair::AllowedAddressPair(const std::string &vrf, uint32_t plen, bool ecmp, const MacAddress &mac) : ListEntry(), vrf_(vrf), addr_(addr), plen_(plen), ecmp_(ecmp), mac_(mac), - l2_entry_installed_(false), ethernet_tag_(0), vrf_ref_(NULL, this), - label_(MplsTable::kInvalidLabel), policy_enabled_nh_(NULL), - policy_disabled_nh_(NULL) { + l2_entry_installed_(false), ecmp_config_changed_(false), ethernet_tag_(0), + vrf_ref_(NULL, this), label_(MplsTable::kInvalidLabel), + policy_enabled_nh_(NULL), policy_disabled_nh_(NULL) { } VmInterface::AllowedAddressPair::~AllowedAddressPair() { @@ -4133,7 +4135,8 @@ void VmInterface::AllowedAddressPair::L2Activate(VmInterface *interface, if (l2_entry_installed_ && force_update == false && policy_change == false && ethernet_tag_ == interface->ethernet_tag() && - old_layer3_forwarding == interface->layer3_forwarding()) { + old_layer3_forwarding == interface->layer3_forwarding() && + ecmp_config_changed_ == false) { return; } @@ -4143,7 +4146,7 @@ void VmInterface::AllowedAddressPair::L2Activate(VmInterface *interface, vrf_ref_ = interface->vrf(); if (old_layer3_forwarding != interface->layer3_forwarding() || - l2_entry_installed_ == false) { + l2_entry_installed_ == false || ecmp_config_changed_) { force_update = true; } @@ -4184,6 +4187,7 @@ void VmInterface::AllowedAddressPair::L2Activate(VmInterface *interface, } else { l2_entry_installed_ = false; } + ecmp_config_changed_ = false; } } @@ -4247,7 +4251,7 @@ void VmInterface::AllowedAddressPair::Activate(VmInterface *interface, IpAddress ip = interface->GetServiceIp(addr_); if (installed_ && force_update == false && policy_change == false && - service_ip_ == ip) { + service_ip_ == ip && ecmp_config_changed_ == false) { return; } @@ -4259,7 +4263,8 @@ void VmInterface::AllowedAddressPair::Activate(VmInterface *interface, if (installed_ == true && policy_change) { InetUnicastAgentRouteTable::ReEvaluatePaths(agent, vrf_, addr_, plen_); - } else if (installed_ == false || force_update || service_ip_ != ip) { + } else if (installed_ == false || force_update || service_ip_ != ip || + ecmp_config_changed_) { service_ip_ = ip; IpAddress dependent_rt; if (ecmp_ == true) { @@ -4290,6 +4295,7 @@ void VmInterface::AllowedAddressPair::Activate(VmInterface *interface, } } installed_ = true; + ecmp_config_changed_ = false; } void VmInterface::AllowedAddressPair::DeActivate(VmInterface *interface) const { @@ -4310,6 +4316,10 @@ void VmInterface::AllowedAddressPairList::Insert(const AllowedAddressPair *rhs) void VmInterface::AllowedAddressPairList::Update(const AllowedAddressPair *lhs, const AllowedAddressPair *rhs) { lhs->set_del_pending(false); + if (lhs->ecmp_ != rhs->ecmp_) { + lhs->ecmp_ = rhs->ecmp_; + lhs->ecmp_config_changed_ = true; + } } void VmInterface::AllowedAddressPairList::Remove(AllowedAddressPairSet::iterator &it) { diff --git a/src/vnsw/agent/oper/vm_interface.h b/src/vnsw/agent/oper/vm_interface.h index 4e25a70b5fb..b44de619657 100644 --- a/src/vnsw/agent/oper/vm_interface.h +++ b/src/vnsw/agent/oper/vm_interface.h @@ -249,9 +249,10 @@ class VmInterface : public Interface { mutable std::string vrf_; IpAddress addr_; uint32_t plen_; - bool ecmp_; + mutable bool ecmp_; MacAddress mac_; mutable bool l2_entry_installed_; + mutable bool ecmp_config_changed_; mutable uint32_t ethernet_tag_; mutable VrfEntryRef vrf_ref_; mutable IpAddress service_ip_;