From 08e043abd1769840bea3854b8d6a2dacb1786840 Mon Sep 17 00:00:00 2001 From: Nischal Sheth Date: Tue, 4 Apr 2017 10:11:18 -0700 Subject: [PATCH] Support router mac in EVPN Type 2 routes Change-Id: I0dc2f5e91d6054ef760d26de220fc7b136f9aaa2 Partial-Bug: 1636654 --- src/bgp/bgp_ribout.cc | 24 +-- src/bgp/bgp_ribout.h | 8 +- src/bgp/bgp_xmpp_channel.cc | 14 ++ src/bgp/test/bgp_xmpp_evpn_test.cc | 167 ++++++++++++++++---- src/bgp/xmpp_message_builder.cc | 2 + src/control-node/test/network_agent_mock.cc | 1 + src/control-node/test/network_agent_mock.h | 9 +- src/schema/xmpp_enet.xsd | 1 + 8 files changed, 179 insertions(+), 47 deletions(-) diff --git a/src/bgp/bgp_ribout.cc b/src/bgp/bgp_ribout.cc index b9ea80d079e..0ff9a296eb0 100644 --- a/src/bgp/bgp_ribout.cc +++ b/src/bgp/bgp_ribout.cc @@ -27,9 +27,10 @@ using std::find; RibOutAttr::NextHop::NextHop(const BgpTable *table, IpAddress address, - uint32_t label, uint32_t l3_label, const ExtCommunity *ext_community, - bool vrf_originated) + const MacAddress &mac, uint32_t label, uint32_t l3_label, + const ExtCommunity *ext_community, bool vrf_originated) : address_(address), + mac_(mac), label_(label), l3_label_(l3_label), origin_vn_index_(-1) { @@ -46,6 +47,8 @@ RibOutAttr::NextHop::NextHop(const BgpTable *table, IpAddress address, int RibOutAttr::NextHop::CompareTo(const NextHop &rhs) const { if (address_ < rhs.address_) return -1; if (address_ > rhs.address_) return 1; + if (mac_ < rhs.mac_) return -1; + if (mac_ > rhs.mac_) return 1; if (label_ < rhs.label_) return -1; if (label_ > rhs.label_) return 1; if (l3_label_ < rhs.l3_label_) return -1; @@ -86,8 +89,9 @@ RibOutAttr::RibOutAttr(const BgpTable *table, const BgpAttr *attr, is_xmpp_(false), vrf_originated_(false) { if (attr) { - nexthop_list_.push_back(NextHop(table, attr->nexthop(), label, l3_label, - attr->ext_community(), false)); + nexthop_list_.push_back(NextHop(table, attr->nexthop(), + attr->mac_address(), label, l3_label, attr->ext_community(), + false)); } } @@ -97,8 +101,9 @@ RibOutAttr::RibOutAttr(const BgpTable *table, const BgpRoute *route, is_xmpp_(is_xmpp), vrf_originated_(route->BestPath()->IsVrfOriginated()) { if (attr && include_nh) { - nexthop_list_.push_back(NextHop(table, attr->nexthop(), label, 0, - attr->ext_community(), vrf_originated_)); + nexthop_list_.push_back(NextHop(table, attr->nexthop(), + attr->mac_address(), label, 0, attr->ext_community(), + vrf_originated_)); } } @@ -138,7 +143,8 @@ RibOutAttr::RibOutAttr(const BgpRoute *route, const BgpAttr *attr, // Remember if the path was originated in the VRF. This is used to // determine if VRF's VN name can be used as the origin VN for the // nexthop. - NextHop nexthop(table, path->GetAttr()->nexthop(), path->GetLabel(), + NextHop nexthop(table, path->GetAttr()->nexthop(), + path->GetAttr()->mac_address(), path->GetLabel(), path->GetL3Label(), path->GetAttr()->ext_community(), path->IsVrfOriginated()); @@ -194,8 +200,8 @@ void RibOutAttr::set_attr(const BgpTable *table, const BgpAttrPtr &attrp, if (!attr_out_) { attr_out_ = attrp; assert(nexthop_list_.empty()); - NextHop nexthop(table, attrp->nexthop(), label, l3_label, - attrp->ext_community(), vrf_originated); + NextHop nexthop(table, attrp->nexthop(), attrp->mac_address(), + label, l3_label, attrp->ext_community(), vrf_originated); nexthop_list_.push_back(nexthop); return; } diff --git a/src/bgp/bgp_ribout.h b/src/bgp/bgp_ribout.h index bf8023c1a2c..4d65d041770 100644 --- a/src/bgp/bgp_ribout.h +++ b/src/bgp/bgp_ribout.h @@ -48,11 +48,12 @@ class RibOutAttr { // the BgpPath originated. A value of -1 means that the VN is unknown. class NextHop { public: - NextHop(const BgpTable *table, IpAddress address, uint32_t label, - uint32_t l3_label, const ExtCommunity *ext_community, - bool vrf_originated); + NextHop(const BgpTable *table, IpAddress address, + const MacAddress &mac, uint32_t label, uint32_t l3_label, + const ExtCommunity *ext_community, bool vrf_originated); const IpAddress address() const { return address_; } + const MacAddress &mac() const { return mac_; } uint32_t label() const { return label_; } uint32_t l3_label() const { return l3_label_; } int origin_vn_index() const { return origin_vn_index_; } @@ -64,6 +65,7 @@ class RibOutAttr { private: IpAddress address_; + MacAddress mac_; uint32_t label_; uint32_t l3_label_; int origin_vn_index_; diff --git a/src/bgp/bgp_xmpp_channel.cc b/src/bgp/bgp_xmpp_channel.cc index 81bc083265e..290bd43980b 100644 --- a/src/bgp/bgp_xmpp_channel.cc +++ b/src/bgp/bgp_xmpp_channel.cc @@ -1654,6 +1654,20 @@ bool BgpXmppChannel::ProcessEnetItem(string vrf_name, nh_address = nhop_address; label = nit->label; l3_label = nit->l3_label; + if (!nit->mac.empty()) { + MacAddress rmac_addr = + MacAddress::FromString(nit->mac, &error); + if (error) { + BGP_LOG_PEER_INSTANCE_WARNING(Peer(), vrf_name, + BGP_LOG_FLAG_ALL, + "Bad next-hop mac address " << nit->mac << + " for enet route " << evpn_prefix.ToXmppIdString()); + return false; + } + RouterMac router_mac(rmac_addr); + ext.communities.push_back( + router_mac.GetExtCommunityValue()); + } } // Process tunnel encapsulation list. diff --git a/src/bgp/test/bgp_xmpp_evpn_test.cc b/src/bgp/test/bgp_xmpp_evpn_test.cc index ac349effe13..c765cc88a5e 100644 --- a/src/bgp/test/bgp_xmpp_evpn_test.cc +++ b/src/bgp/test/bgp_xmpp_evpn_test.cc @@ -1414,13 +1414,16 @@ class BgpXmppEvpnTest2 : public ::testing::Test { return true; } - bool CheckRouteLabels(test::NetworkAgentMockPtr agent, - const string &network, const string &prefix, int label, int l3_label) { + bool CheckRouteMacLabels(test::NetworkAgentMockPtr agent, + const string &network, const string &prefix, const string &mac, + int label, int l3_label) { task_util::TaskSchedulerLock lock; const autogen::EnetItemType *rt = agent->EnetRouteLookup(network, prefix); if (!rt) return false; + if (!mac.empty() && rt->entry.next_hops.next_hop[0].mac != mac) + return false; if (label && rt->entry.next_hops.next_hop[0].label != label) return false; if (l3_label && rt->entry.next_hops.next_hop[0].l3_label != l3_label) @@ -1464,10 +1467,11 @@ class BgpXmppEvpnTest2 : public ::testing::Test { TASK_UTIL_EXPECT_TRUE(CheckRouteExists(agent, network, prefix)); } - void VerifyRouteLabels(test::NetworkAgentMockPtr agent, - const string &network, const string &prefix, int label, int l3_label) { + void VerifyRouteMacLabels(test::NetworkAgentMockPtr agent, + const string &network, const string &prefix, const string &mac, + int label, int l3_label) { TASK_UTIL_EXPECT_TRUE( - CheckRouteLabels(agent, network, prefix, label, l3_label)); + CheckRouteMacLabels(agent, network, prefix, mac, label, l3_label)); } void VerifyRouteSecurityGroup(test::NetworkAgentMockPtr agent, @@ -2032,9 +2036,9 @@ TEST_F(BgpXmppEvpnTest2, RouteUpdate) { } // -// Routes from 2 agents are advertised with L3 label. +// Routes from 2 agents are advertised with router MAC and L3 label. // -TEST_F(BgpXmppEvpnTest2, RouteAddL3Label) { +TEST_F(BgpXmppEvpnTest2, RouteAddRouterMacL3Label) { Configure(); task_util::WaitForIdle(); @@ -2055,17 +2059,19 @@ TEST_F(BgpXmppEvpnTest2, RouteAddL3Label) { agent_b_->EnetSubscribe("blue", 1); task_util::WaitForIdle(); - // Add route from agent A with label 101 and l3-label 200. + // Add route from agent A with mac mac_a, label 101 and l3-label 200. stringstream eroute_a; eroute_a << "aa:00:00:00:00:01,10.1.1.1/32"; - test::NextHop nh_a("192.168.1.1", 101, 200); + string mac_a("aa:aa:aa:00:00:01"); + test::NextHop nh_a("192.168.1.1", mac_a, 101, 200); agent_a_->AddEnetRoute("blue", eroute_a.str(), nh_a); task_util::WaitForIdle(); - // Add route from agent B with label 102 and l3-label 200.. + // Add route from agent B with mac mac_b, label 102 and l3-label 200. stringstream eroute_b; eroute_b << "bb:00:00:00:00:01,10.1.2.1/32"; - test::NextHop nh_b("192.168.2.1", 102, 200); + string mac_b("bb:bb:bb:00:00:01"); + test::NextHop nh_b("192.168.2.1", mac_b, 102, 200); agent_b_->AddEnetRoute("blue", eroute_b.str(), nh_b); task_util::WaitForIdle(); @@ -2075,11 +2081,11 @@ TEST_F(BgpXmppEvpnTest2, RouteAddL3Label) { TASK_UTIL_EXPECT_EQ(2, agent_b_->EnetRouteCount()); TASK_UTIL_EXPECT_EQ(2, agent_b_->EnetRouteCount("blue")); - // Verify label and l3-label values. - VerifyRouteLabels(agent_a_, "blue", eroute_a.str(), 101, 200); - VerifyRouteLabels(agent_a_, "blue", eroute_b.str(), 102, 200); - VerifyRouteLabels(agent_b_, "blue", eroute_a.str(), 101, 200); - VerifyRouteLabels(agent_b_, "blue", eroute_b.str(), 102, 200); + // Verify mac, label and l3-label values. + VerifyRouteMacLabels(agent_a_, "blue", eroute_a.str(), mac_a, 101, 200); + VerifyRouteMacLabels(agent_a_, "blue", eroute_b.str(), mac_b, 102, 200); + VerifyRouteMacLabels(agent_b_, "blue", eroute_a.str(), mac_a, 101, 200); + VerifyRouteMacLabels(agent_b_, "blue", eroute_b.str(), mac_b, 102, 200); // Delete route from agent A. agent_a_->DeleteEnetRoute("blue", eroute_a.str()); @@ -2124,17 +2130,19 @@ TEST_F(BgpXmppEvpnTest2, RouteUpdateL3Label) { agent_b_->EnetSubscribe("blue", 1); task_util::WaitForIdle(); - // Add route from agent A with label 101 and l3-label 200. + // Add route from agent A with mac mac_a, label 101 and l3-label 200. stringstream eroute_a; eroute_a << "aa:00:00:00:00:01,10.1.1.1/32"; - test::NextHop nh_a("192.168.1.1", 101, 200); + string mac_a("aa:aa:aa:00:00:01"); + test::NextHop nh_a("192.168.1.1", mac_a, 101, 200); agent_a_->AddEnetRoute("blue", eroute_a.str(), nh_a); task_util::WaitForIdle(); - // Add route from agent B with label 102 and l3-label 200.. + // Add route from agent B with mac mac_b, label 102 and l3-label 200.. stringstream eroute_b; eroute_b << "bb:00:00:00:00:01,10.1.2.1/32"; - test::NextHop nh_b("192.168.2.1", 102, 200); + string mac_b("bb:bb:bb:00:00:01"); + test::NextHop nh_b("192.168.2.1", mac_b, 102, 200); agent_b_->AddEnetRoute("blue", eroute_b.str(), nh_b); task_util::WaitForIdle(); @@ -2144,19 +2152,114 @@ TEST_F(BgpXmppEvpnTest2, RouteUpdateL3Label) { TASK_UTIL_EXPECT_EQ(2, agent_b_->EnetRouteCount()); TASK_UTIL_EXPECT_EQ(2, agent_b_->EnetRouteCount("blue")); - // Verify label and l3-label values. - VerifyRouteLabels(agent_a_, "blue", eroute_a.str(), 101, 200); - VerifyRouteLabels(agent_a_, "blue", eroute_b.str(), 102, 200); - VerifyRouteLabels(agent_b_, "blue", eroute_a.str(), 101, 200); - VerifyRouteLabels(agent_b_, "blue", eroute_b.str(), 102, 200); + // Verify mac, label and l3-label values. + VerifyRouteMacLabels(agent_a_, "blue", eroute_a.str(), mac_a, 101, 200); + VerifyRouteMacLabels(agent_a_, "blue", eroute_b.str(), mac_b, 102, 200); + VerifyRouteMacLabels(agent_b_, "blue", eroute_a.str(), mac_a, 101, 200); + VerifyRouteMacLabels(agent_b_, "blue", eroute_b.str(), mac_b, 102, 200); // Update l3-label of route from agent A to 300. - nh_a = test::NextHop("192.168.1.1", 101, 300); + nh_a = test::NextHop("192.168.1.1", mac_a, 101, 300); agent_a_->AddEnetRoute("blue", eroute_a.str(), nh_a); task_util::WaitForIdle(); // Update l3-label of route from agent B to 300. - nh_b = test::NextHop("192.168.2.1", 102, 300); + nh_b = test::NextHop("192.168.2.1", mac_b, 102, 300); + agent_b_->AddEnetRoute("blue", eroute_b.str(), nh_b); + task_util::WaitForIdle(); + + // Verify that routes showed up on the agents. + TASK_UTIL_EXPECT_EQ(2, agent_a_->EnetRouteCount()); + TASK_UTIL_EXPECT_EQ(2, agent_a_->EnetRouteCount("blue")); + TASK_UTIL_EXPECT_EQ(2, agent_b_->EnetRouteCount()); + TASK_UTIL_EXPECT_EQ(2, agent_b_->EnetRouteCount("blue")); + + // Verify mac, label and l3-label values. + VerifyRouteMacLabels(agent_a_, "blue", eroute_a.str(), mac_a, 101, 300); + VerifyRouteMacLabels(agent_a_, "blue", eroute_b.str(), mac_b, 102, 300); + VerifyRouteMacLabels(agent_b_, "blue", eroute_a.str(), mac_a, 101, 300); + VerifyRouteMacLabels(agent_b_, "blue", eroute_b.str(), mac_b, 102, 300); + + // Delete route from agent A. + agent_a_->DeleteEnetRoute("blue", eroute_a.str()); + task_util::WaitForIdle(); + + // Delete route from agent B. + agent_b_->DeleteEnetRoute("blue", eroute_b.str()); + task_util::WaitForIdle(); + + // Verify that there are no routes on the agents. + TASK_UTIL_EXPECT_EQ(0, agent_a_->EnetRouteCount()); + TASK_UTIL_EXPECT_EQ(0, agent_a_->EnetRouteCount("blue")); + TASK_UTIL_EXPECT_EQ(0, agent_b_->EnetRouteCount()); + TASK_UTIL_EXPECT_EQ(0, agent_b_->EnetRouteCount("blue")); + + // Close the sessions. + agent_a_->SessionDown(); + agent_b_->SessionDown(); +} + +// +// Routes from 2 agents are updated when router MAC is updated. +// +TEST_F(BgpXmppEvpnTest2, RouteUpdateRouterMac) { + Configure(); + task_util::WaitForIdle(); + + // Create XMPP Agent A connected to XMPP server X. + agent_a_.reset( + new test::NetworkAgentMock(&evm_, "agent-a", xs_x_->GetPort(), + "127.0.0.1", "127.0.0.1")); + TASK_UTIL_EXPECT_TRUE(agent_a_->IsEstablished()); + + // Create XMPP Agent B connected to XMPP server Y. + agent_b_.reset( + new test::NetworkAgentMock(&evm_, "agent-b", xs_y_->GetPort(), + "127.0.0.2", "127.0.0.2")); + TASK_UTIL_EXPECT_TRUE(agent_b_->IsEstablished()); + + // Register to blue instance + agent_a_->EnetSubscribe("blue", 1); + agent_b_->EnetSubscribe("blue", 1); + task_util::WaitForIdle(); + + // Add route from agent A with mac mac_a1, label 101 and l3-label 200. + stringstream eroute_a; + eroute_a << "aa:00:00:00:00:01,10.1.1.1/32"; + string mac_a1("aa:aa:aa:00:00:01"); + test::NextHop nh_a("192.168.1.1", mac_a1, 101, 200); + agent_a_->AddEnetRoute("blue", eroute_a.str(), nh_a); + task_util::WaitForIdle(); + + // Add route from agent B with mac mac_b1, label 102 and l3-label 200. + stringstream eroute_b; + eroute_b << "bb:00:00:00:00:01,10.1.2.1/32"; + string mac_b1("bb:bb:bb:00:00:01"); + test::NextHop nh_b("192.168.2.1", mac_b1, 102, 200); + agent_b_->AddEnetRoute("blue", eroute_b.str(), nh_b); + task_util::WaitForIdle(); + + // Verify that routes showed up on the agents. + TASK_UTIL_EXPECT_EQ(2, agent_a_->EnetRouteCount()); + TASK_UTIL_EXPECT_EQ(2, agent_a_->EnetRouteCount("blue")); + TASK_UTIL_EXPECT_EQ(2, agent_b_->EnetRouteCount()); + TASK_UTIL_EXPECT_EQ(2, agent_b_->EnetRouteCount("blue")); + + // Verify mac, label and l3-label values. + VerifyRouteMacLabels(agent_a_, "blue", eroute_a.str(), mac_a1, 101, 200); + VerifyRouteMacLabels(agent_a_, "blue", eroute_b.str(), mac_b1, 102, 200); + VerifyRouteMacLabels(agent_b_, "blue", eroute_a.str(), mac_a1, 101, 200); + VerifyRouteMacLabels(agent_b_, "blue", eroute_b.str(), mac_b1, 102, 200); + + // Update mac of route from agent A to mac_a2. + string mac_a2("aa:aa:aa:00:00:02"); + nh_a = test::NextHop("192.168.1.1", mac_a2, 101, 200); + agent_a_->AddEnetRoute("blue", eroute_a.str(), nh_a); + task_util::WaitForIdle(); + + // Update mac of route from agent B to mac_b2. + string mac_b2("bb:bb:bb:00:00:02"); + nh_b = test::NextHop("192.168.2.1", mac_b2, 102, 200); agent_b_->AddEnetRoute("blue", eroute_b.str(), nh_b); task_util::WaitForIdle(); @@ -2166,11 +2269,11 @@ TEST_F(BgpXmppEvpnTest2, RouteUpdateL3Label) { TASK_UTIL_EXPECT_EQ(2, agent_b_->EnetRouteCount()); TASK_UTIL_EXPECT_EQ(2, agent_b_->EnetRouteCount("blue")); - // Verify label and l3-label values. - VerifyRouteLabels(agent_a_, "blue", eroute_a.str(), 101, 300); - VerifyRouteLabels(agent_a_, "blue", eroute_b.str(), 102, 300); - VerifyRouteLabels(agent_b_, "blue", eroute_a.str(), 101, 300); - VerifyRouteLabels(agent_b_, "blue", eroute_b.str(), 102, 300); + // Verify mac, label and l3-label values. + VerifyRouteMacLabels(agent_a_, "blue", eroute_a.str(), mac_a2, 101, 200); + VerifyRouteMacLabels(agent_a_, "blue", eroute_b.str(), mac_b2, 102, 200); + VerifyRouteMacLabels(agent_b_, "blue", eroute_a.str(), mac_a2, 101, 200); + VerifyRouteMacLabels(agent_b_, "blue", eroute_b.str(), mac_b2, 102, 200); // Delete route from agent A. agent_a_->DeleteEnetRoute("blue", eroute_a.str()); diff --git a/src/bgp/xmpp_message_builder.cc b/src/bgp/xmpp_message_builder.cc index 690a44180a6..72d5462f7c1 100644 --- a/src/bgp/xmpp_message_builder.cc +++ b/src/bgp/xmpp_message_builder.cc @@ -280,6 +280,8 @@ void BgpXmppMessage::EncodeEnetNextHop(const BgpRoute *route, item_nexthop.address = nexthop.address().to_v4().to_string(); item_nexthop.label = nexthop.label(); item_nexthop.l3_label = nexthop.l3_label(); + if (!nexthop.mac().IsZero()) + item_nexthop.mac = nexthop.mac().ToString(); // If encap list is empty use mpls over gre as default encap. vector &encap_list = diff --git a/src/control-node/test/network_agent_mock.cc b/src/control-node/test/network_agent_mock.cc index dbcadbae15a..4c764244b04 100644 --- a/src/control-node/test/network_agent_mock.cc +++ b/src/control-node/test/network_agent_mock.cc @@ -658,6 +658,7 @@ pugi::xml_document *XmppDocumentMock::RouteEnetAddDeleteXmlDoc( autogen::EnetNextHopType item_nexthop; item_nexthop.af = BgpAf::IPv4; item_nexthop.address = nexthop.address_; + item_nexthop.mac = nexthop.mac_; item_nexthop.label = nexthop.label_ ? nexthop.label_ : label_alloc_++; item_nexthop.l3_label = nexthop.l3_label_; diff --git a/src/control-node/test/network_agent_mock.h b/src/control-node/test/network_agent_mock.h index a02f86035a0..163247db794 100644 --- a/src/control-node/test/network_agent_mock.h +++ b/src/control-node/test/network_agent_mock.h @@ -207,17 +207,19 @@ struct NextHop { tunnel_encapsulations_.push_back(tunnel); } } - NextHop(std::string address, uint32_t label, uint32_t l3_label, - const std::string virtual_network = "") : - address_(address), no_label_(false), label_(label), + NextHop(std::string address, std::string mac, uint32_t label, + uint32_t l3_label, const std::string virtual_network = "") : + address_(address), mac_(mac), no_label_(false), label_(label), l3_label_(l3_label), virtual_network_(virtual_network) { tunnel_encapsulations_.push_back("vxlan"); } bool operator==(NextHop other) { if (address_ != other.address_) return false; + if (mac_ != other.mac_) return false; if (no_label_ != other.no_label_) return false; if (label_ != other.label_) return false; + if (l3_label_ != other.l3_label_) return false; if (tunnel_encapsulations_.size() != other.tunnel_encapsulations_.size()) { return false; @@ -237,6 +239,7 @@ struct NextHop { } std::string address_; + std::string mac_; bool no_label_; int label_; int l3_label_; diff --git a/src/schema/xmpp_enet.xsd b/src/schema/xmpp_enet.xsd index 0e41248c1ae..d48fd7a8e31 100644 --- a/src/schema/xmpp_enet.xsd +++ b/src/schema/xmpp_enet.xsd @@ -22,6 +22,7 @@ xsd:targetNamespace="http://www.contrailsystems.com/xmpp-enet-cfg.xsd"> +