diff --git a/src/bgp/routing-policy/routing_policy_action.cc b/src/bgp/routing-policy/routing_policy_action.cc index 3f36bdd0d95..f5233474a46 100644 --- a/src/bgp/routing-policy/routing_policy_action.cc +++ b/src/bgp/routing-policy/routing_policy_action.cc @@ -41,20 +41,18 @@ UpdateCommunity::UpdateCommunity(const std::vector communities, void UpdateCommunity::operator()(BgpAttr *attr) const { if (!attr) return; const Community *comm = attr->community(); - if (comm) { - BgpAttrDB *attr_db = attr->attr_db(); - BgpServer *server = attr_db->server(); - CommunityDB *comm_db = server->comm_db(); - CommunityPtr new_community; - if (op_ == SET) { - new_community = comm_db->SetAndLocate(comm, communities_); - } else if (op_ == ADD) { - new_community = comm_db->AppendAndLocate(comm, communities_); - } else if (op_ == REMOVE) { - new_community = comm_db->RemoveAndLocate(comm, communities_); - } - attr->set_community(new_community); + BgpAttrDB *attr_db = attr->attr_db(); + BgpServer *server = attr_db->server(); + CommunityDB *comm_db = server->comm_db(); + CommunityPtr new_community = NULL; + if (op_ == SET) { + new_community = comm_db->SetAndLocate(comm, communities_); + } else if (op_ == ADD) { + new_community = comm_db->AppendAndLocate(comm, communities_); + } else if (op_ == REMOVE) { + if (comm) new_community = comm_db->RemoveAndLocate(comm, communities_); } + attr->set_community(new_community); } string UpdateCommunity::ToString() const { diff --git a/src/bgp/test/routing_policy_test.cc b/src/bgp/test/routing_policy_test.cc index 535f4a0aa4c..2bc5077db83 100644 --- a/src/bgp/test/routing_policy_test.cc +++ b/src/bgp/test/routing_policy_test.cc @@ -537,8 +537,7 @@ TEST_F(RoutingPolicyTest, PolicyPrefixMatchSetCommunity) { peers_.push_back( new BgpPeerMock(Ip4Address::from_string("192.168.0.1", ec))); - AddRoute(peers_[0], "test.inet.0", "1.1.1.1/32", 100, - list_of("23:13")("23:44")); + AddRoute(peers_[0], "test.inet.0", "1.1.1.1/32", 100); task_util::WaitForIdle(); VERIFY_EQ(1, RouteCount("test.inet.0")); @@ -549,7 +548,7 @@ TEST_F(RoutingPolicyTest, PolicyPrefixMatchSetCommunity) { ASSERT_TRUE(rt->BestPath()->IsFeasible() == true); ASSERT_EQ(GetCommunityListFromRoute(rt->BestPath()), list_of("11:13")); ASSERT_EQ(GetOriginalCommunityListFromRoute(rt->BestPath()), - list_of("23:13")("23:44")); + vector()); DeleteRoute(peers_[0], "test.inet.0", "1.1.1.1/32"); } @@ -607,6 +606,32 @@ TEST_F(RoutingPolicyTest, PolicyPrefixMatchRemoveMultipleCommunity_1) { DeleteRoute(peers_[0], "test.inet.0", "1.1.1.1/32"); } +// Input route has no community to begin with +TEST_F(RoutingPolicyTest, PolicyPrefixMatchRemoveMultipleCommunity_2) { + string content = + FileRead("controller/src/bgp/testdata/routing_policy_2d.xml"); + EXPECT_TRUE(parser_.Parse(content)); + task_util::WaitForIdle(); + + boost::system::error_code ec; + peers_.push_back( + new BgpPeerMock(Ip4Address::from_string("192.168.0.1", ec))); + + AddRoute(peers_[0], "test.inet.0", "1.1.1.1/32", 100); + task_util::WaitForIdle(); + + VERIFY_EQ(1, RouteCount("test.inet.0")); + BgpRoute *rt = + RouteLookup("test.inet.0", "1.1.1.1/32"); + ASSERT_TRUE(rt != NULL); + VERIFY_EQ(peers_[0], rt->BestPath()->GetPeer()); + ASSERT_TRUE(rt->BestPath()->IsFeasible() == true); + ASSERT_EQ(GetCommunityListFromRoute(rt->BestPath()), vector()); + ASSERT_EQ(GetOriginalCommunityListFromRoute(rt->BestPath()), + vector()); + DeleteRoute(peers_[0], "test.inet.0", "1.1.1.1/32"); +} + TEST_F(RoutingPolicyTest, PolicyPrefixMatchAddMultipleCommunity) { string content = FileRead("controller/src/bgp/testdata/routing_policy_2e.xml"); @@ -634,6 +659,34 @@ TEST_F(RoutingPolicyTest, PolicyPrefixMatchAddMultipleCommunity) { DeleteRoute(peers_[0], "test.inet.0", "1.1.1.1/32"); } +// Route has not community to start with +TEST_F(RoutingPolicyTest, PolicyPrefixMatchAddMultipleCommunity_1) { + string content = + FileRead("controller/src/bgp/testdata/routing_policy_2e.xml"); + EXPECT_TRUE(parser_.Parse(content)); + task_util::WaitForIdle(); + + boost::system::error_code ec; + peers_.push_back( + new BgpPeerMock(Ip4Address::from_string("192.168.0.1", ec))); + + AddRoute(peers_[0], "test.inet.0", "1.1.1.1/32", 100); + task_util::WaitForIdle(); + + VERIFY_EQ(1, RouteCount("test.inet.0")); + BgpRoute *rt = + RouteLookup("test.inet.0", "1.1.1.1/32"); + ASSERT_TRUE(rt != NULL); + VERIFY_EQ(peers_[0], rt->BestPath()->GetPeer()); + ASSERT_TRUE(rt->BestPath()->IsFeasible() == true); + ASSERT_EQ(GetCommunityListFromRoute(rt->BestPath()), + list_of("11:22")("22:44")("44:88")); + ASSERT_EQ(GetOriginalCommunityListFromRoute(rt->BestPath()), + vector()); + DeleteRoute(peers_[0], "test.inet.0", "1.1.1.1/32"); +} + + TEST_F(RoutingPolicyTest, PolicyPrefixMatchSetMultipleCommunity) { string content = FileRead("controller/src/bgp/testdata/routing_policy_2f.xml");