diff --git a/src/vnsw/agent/oper/agent_path.cc b/src/vnsw/agent/oper/agent_path.cc index c92ee73a9d3..6a6348146aa 100644 --- a/src/vnsw/agent/oper/agent_path.cc +++ b/src/vnsw/agent/oper/agent_path.cc @@ -871,7 +871,9 @@ bool MulticastRoute::CopyPathParameters(Agent *agent, path->set_dest_vn_name(vn_name); path->set_unresolved(unresolved); path->set_vxlan_id(vxlan_id); - path->set_label(label); + if ((path->peer() != agent->local_vm_peer()) && + (path->peer() != agent->local_peer())) + path->set_label(label); //Setting of tunnel is only for simulated TOR. path->set_tunnel_bmap(tunnel_type); diff --git a/src/vnsw/agent/oper/bridge_route.cc b/src/vnsw/agent/oper/bridge_route.cc index b7a1e168eb4..840998f1726 100644 --- a/src/vnsw/agent/oper/bridge_route.cc +++ b/src/vnsw/agent/oper/bridge_route.cc @@ -571,6 +571,89 @@ bool BridgeRouteEntry::ReComputePathDeletion(AgentPath *path) { return false; } +void BridgeRouteEntry::HandleMulticastLabel(const Agent *agent, + AgentPath *path, + const AgentPath *local_peer_path, + const AgentPath *local_vm_peer_path, + bool del, + uint32_t *evpn_label) { + *evpn_label = MplsTable::kInvalidLabel; + + //EVPN label is present in two paths: + // local_vm_peer(courtesy: vmi) or local_peer(courtesy: vn) + // Irrespective of delete/add operation if one of them is present and is not + // the affected path, then extract the label from same. + // By default pick it from available path (local or local_vm). + switch (path->peer()->GetType()) { + case Peer::LOCAL_VM_PEER: + //Use local_peer path for label + if (local_peer_path) { + *evpn_label = local_peer_path->label(); + assert(*evpn_label != MplsTable::kInvalidLabel); + } + break; + case Peer::LOCAL_PEER: + //Use local_peer path for label + if (local_vm_peer_path) { + *evpn_label = local_vm_peer_path->label(); + assert(*evpn_label != MplsTable::kInvalidLabel); + } + break; + default: + if (local_vm_peer_path) { + *evpn_label = local_vm_peer_path->label(); + assert(*evpn_label != MplsTable::kInvalidLabel); + } else if (local_peer_path) { + *evpn_label = local_peer_path->label(); + assert(*evpn_label != MplsTable::kInvalidLabel); + } + break; + } + + //Delete path evpn label if path is local_peer or local_vm_peer. + //Delete fabric label if path is multicast_fabric_tree + if (del) { + bool delete_label = false; + // On deletion of fabric path delete fabric label. + // Other type of label is evpn mcast label. + // EVPN label is deleted when both local peer and local_vm_peer path are + // gone. + if (path->peer()->GetType() == Peer::MULTICAST_FABRIC_TREE_BUILDER) + delete_label = true; + else if ((path->peer() == agent->local_vm_peer()) || + (path->peer() == agent->local_peer())) { + if (local_peer_path == NULL && + local_vm_peer_path == NULL) + delete_label = true; + } + if (delete_label) + agent->mpls_table()->DeleteMcastLabel(path->label()); + + return; + } + + // Currently other than evpn label no other multicast path requires dynamic + // allocation so return. + if ((path != local_peer_path) && (path != local_vm_peer_path)) + return; + + // Path already has label, return. + if (path->label() != MplsTable::kInvalidLabel) + return; + + // If this is the first time i.e. local_peer has come with no local_vm_peer + // and vice versa then allocate label. + // If its not then we should have valid evpn label calculated above. + if (*evpn_label == MplsTable::kInvalidLabel) { + // XOR use - we shud never reach here when both are NULL or set. + // Only one should be present. + assert((local_vm_peer_path != NULL) ^ (local_peer_path != NULL)); + *evpn_label = agent->mpls_table()->AllocLabel(); + } + assert(*evpn_label != MplsTable::kInvalidLabel); + path->set_label(*evpn_label); +} + bool BridgeRouteEntry::ReComputeMulticastPaths(AgentPath *path, bool del) { if (path->peer() == NULL) { return false; @@ -600,13 +683,9 @@ bool BridgeRouteEntry::ReComputeMulticastPaths(AgentPath *path, bool del) { AgentPath *evpn_peer_path = NULL; AgentPath *fabric_peer_path = NULL; AgentPath *tor_peer_path = NULL; + AgentPath *local_peer_path = NULL; bool tor_path = false; - //Delete path label - if (del && (path->peer()->GetType() != Peer::BGP_PEER)) { - agent->mpls_table()->DeleteMcastLabel(path->label()); - } - const CompositeNH *cnh = static_cast(path->nexthop()); if (cnh && (cnh->composite_nh_type() == Composite::TOR)) { @@ -659,6 +738,8 @@ bool BridgeRouteEntry::ReComputeMulticastPaths(AgentPath *path, bool del) { fabric_peer_path = it_path; } else if (it_path->peer() == agent->multicast_peer()) { multicast_peer_path = it_path; + } else if (it_path->peer() == agent->local_peer()) { + local_peer_path = it_path; } } @@ -668,6 +749,13 @@ bool BridgeRouteEntry::ReComputeMulticastPaths(AgentPath *path, bool del) { } } + uint32_t evpn_label = MplsTable::kInvalidLabel; + HandleMulticastLabel(agent, path, + local_peer_path, + local_vm_peer_path, + del, + &evpn_label); + //all paths are gone so delete multicast_peer path as well if ((local_vm_peer_path == NULL) && (tor_peer_path == NULL) && @@ -746,7 +834,6 @@ bool BridgeRouteEntry::ReComputeMulticastPaths(AgentPath *path, bool del) { bool unresolved = false; uint32_t vxlan_id = 0; uint32_t label = 0; - uint32_t evpn_label = 0; uint32_t tunnel_bmap = TunnelType::AllType(); //Select based on priority of path peer. @@ -756,7 +843,6 @@ bool BridgeRouteEntry::ReComputeMulticastPaths(AgentPath *path, bool del) { vxlan_id = local_vm_peer_path->vxlan_id(); tunnel_bmap = TunnelType::AllType(); label = local_vm_peer_path->label(); - evpn_label = label; } else if (tor_peer_path) { dest_vn_name = tor_peer_path->dest_vn_name(); unresolved = tor_peer_path->unresolved(); @@ -791,6 +877,7 @@ bool BridgeRouteEntry::ReComputeMulticastPaths(AgentPath *path, bool del) { label, tunnel_bmap, nh); + //Bake all MPLS label if (fabric_peer_path) { //Add new label @@ -804,8 +891,8 @@ bool BridgeRouteEntry::ReComputeMulticastPaths(AgentPath *path, bool del) { } } - //Populate label only when local vm peer path is present. - if (evpn_label != 0 && local_vm_peer_path) { + // Rebake label with whatever comp NH has been calculated. + if (evpn_label != MplsTable::kInvalidLabel) { agent->mpls_table()->CreateMcastLabel(evpn_label, Composite::L2COMP, component_nh_list, diff --git a/src/vnsw/agent/oper/bridge_route.h b/src/vnsw/agent/oper/bridge_route.h index 2d1d474f92e..6c1a45186de 100644 --- a/src/vnsw/agent/oper/bridge_route.h +++ b/src/vnsw/agent/oper/bridge_route.h @@ -144,6 +144,11 @@ class BridgeRouteEntry : public AgentRoute { const AgentPath *FindOvsPath() const; private: + void HandleMulticastLabel(const Agent *agent, + AgentPath *path, + const AgentPath *local_peer_path, + const AgentPath *local_vm_peer_path, + bool del, uint32_t *evpn_label); bool ReComputeMulticastPaths(AgentPath *path, bool del); AgentPath *FindEvpnPathUsingKeyData(const AgentRouteKey *key, const AgentRouteData *data) const; diff --git a/src/vnsw/agent/oper/multicast.cc b/src/vnsw/agent/oper/multicast.cc index 685e2c189e8..f598c939805 100644 --- a/src/vnsw/agent/oper/multicast.cc +++ b/src/vnsw/agent/oper/multicast.cc @@ -116,7 +116,7 @@ void MulticastHandler::HandleVxLanChange(const VnEntry *vn) { obj->set_vxlan_id(new_vxlan_id); //Rebake new vxlan id in mcast route AddL2BroadcastRoute(obj, vn->GetVrf()->GetName(), vn->GetName(), - broadcast, obj->evpn_mpls_label(), new_vxlan_id, 0); + broadcast, MplsTable::kInvalidLabel, new_vxlan_id, 0); } } @@ -139,11 +139,11 @@ void MulticastHandler::HandleVnParametersChange(DBTablePartBase *partition, boost::system::error_code ec; Ip4Address broadcast = IpAddress::from_string("255.255.255.255", ec).to_v4(); - //Add operation if (!deleted) { MulticastGroupObject *all_broadcast = FindFloodGroupObject(vn->GetVrf()->GetName()); + if (!state) { state = new MulticastDBState(vn->GetVrf()->GetName(), vn_vxlan_id); @@ -152,9 +152,10 @@ void MulticastHandler::HandleVnParametersChange(DBTablePartBase *partition, state); //Also create multicast object if (all_broadcast == NULL) { - CreateMulticastGroupObject(state->vrf_name_, broadcast, - vn->GetName(), state->vxlan_id_); + all_broadcast = CreateMulticastGroupObject(state->vrf_name_, broadcast, + vn, state->vxlan_id_); } + all_broadcast->set_vn(vn); } else { if (old_vxlan_id != vn_vxlan_id) { state->vxlan_id_ = vn_vxlan_id; @@ -183,9 +184,13 @@ void MulticastHandler::HandleVnParametersChange(DBTablePartBase *partition, if (!state) return; - if (deleted) { - DeleteMulticastObject(state->vrf_name_, broadcast); + MulticastGroupObject *all_broadcast = + FindFloodGroupObject(state->vrf_name_); + if (all_broadcast) { + all_broadcast->reset_vn(); } + + DeleteMulticastObject(state->vrf_name_, broadcast); BridgeAgentRouteTable::DeleteBroadcastReq(agent_->local_peer(), state->vrf_name_, old_vxlan_id, @@ -206,19 +211,18 @@ void MulticastHandler::ModifyVN(DBTablePartBase *partition, DBEntryBase *e) } bool MulticastGroupObject::CanBeDeleted() const { - if (local_olist_.size() == 0) + if ((local_olist_.size() == 0) && (vn_.get() == NULL)) return true; return false; } MulticastGroupObject *MulticastHandler::CreateMulticastGroupObject -(const string &vrf_name, const Ip4Address &ip_addr, const string &vn_name, +(const string &vrf_name, const Ip4Address &ip_addr, const VnEntry *vn, uint32_t vxlan_id) { MulticastGroupObject *obj = - new MulticastGroupObject(vrf_name, ip_addr, vn_name); + new MulticastGroupObject(vrf_name, ip_addr, vn->GetName()); obj->set_vxlan_id(vxlan_id); AddToMulticastObjList(obj); - obj->CreateEvpnMplsLabel(agent_); return obj; } @@ -384,18 +388,12 @@ MulticastGroupObject *MulticastHandler::FindActiveGroupObject( return obj; } -void MulticastGroupObject::CreateEvpnMplsLabel(const Agent *agent) { - //Already added - if ((evpn_mpls_label_ != MplsLabel::INVALID) && - (evpn_mpls_label_ != 0)) { - return; - } +void MulticastGroupObject::set_vn(const VnEntry *vn) { + vn_.reset(vn); +} - evpn_mpls_label_ = agent->mpls_table()->AllocLabel(); - if (evpn_mpls_label_ == MplsLabel::INVALID) { - MCTRACE(Log, "allocation of evpn mpls label failed", - vrf_name_, "FF:FF:FF:FF:FF:FF", 0); - } +void MulticastGroupObject::reset_vn() { + vn_.reset(); } ComponentNHKeyList @@ -427,7 +425,7 @@ void MulticastHandler::TriggerLocalRouteChange(MulticastGroupObject *obj, AgentRouteData *data = BridgeAgentRouteTable::BuildNonBgpPeerData(obj->vrf_name(), obj->GetVnName(), - obj->evpn_mpls_label(), + MplsTable::kInvalidLabel, obj->vxlan_id(), route_tunnel_bmap, Composite::L2INTERFACE, @@ -577,7 +575,7 @@ void MulticastHandler::AddVmInterfaceInFloodGroup(const VmInterface *vm_itf) { all_broadcast = this->FindGroupObject(vrf_name, broadcast); if (all_broadcast == NULL) { all_broadcast = CreateMulticastGroupObject(vrf_name, broadcast, - vn_name, vn->GetVxLanId()); + vn, vn->GetVxLanId()); add_route = true; } @@ -660,8 +658,7 @@ void MulticastHandler::ModifyEvpnMembers(const Peer *peer, TriggerRemoteRouteChange(obj, peer, vrf_name, olist, peer_identifier, delete_op, Composite::EVPN, - (obj ? obj->evpn_mpls_label() : 0), false, - ethernet_tag); + ethernet_tag, false, ethernet_tag); MCTRACE(Log, "Add EVPN TOR Olist ", vrf_name, grp.to_string(), 0); } @@ -686,8 +683,7 @@ void MulticastHandler::ModifyTorMembers(const Peer *peer, TriggerRemoteRouteChange(obj, peer, vrf_name, olist, peer_identifier, delete_op, Composite::TOR, - (obj ? obj->evpn_mpls_label() : 0), false, - ethernet_tag); + MplsTable::kInvalidLabel, false, ethernet_tag); MCTRACE(Log, "Add external TOR Olist ", vrf_name, grp.to_string(), 0); } diff --git a/src/vnsw/agent/oper/multicast.h b/src/vnsw/agent/oper/multicast.h index 18927e20391..330ce9b5ef1 100644 --- a/src/vnsw/agent/oper/multicast.h +++ b/src/vnsw/agent/oper/multicast.h @@ -55,8 +55,7 @@ class MulticastGroupObject { const Ip4Address &grp_addr, const std::string &vn_name) : vrf_name_(vrf_name), grp_address_(grp_addr), vn_name_(vn_name), - evpn_mpls_label_(0), vxlan_id_(0), peer_identifier_(0), - deleted_(false) { + vxlan_id_(0), peer_identifier_(0), deleted_(false), vn_(NULL) { boost::system::error_code ec; src_address_ = IpAddress::from_string("0.0.0.0", ec).to_v4(); local_olist_.clear(); @@ -65,15 +64,12 @@ class MulticastGroupObject { const Ip4Address &grp_addr, const Ip4Address &src_addr) : vrf_name_(vrf_name), grp_address_(grp_addr), src_address_(src_addr), - evpn_mpls_label_(0), vxlan_id_(0), peer_identifier_(0), - deleted_(false) { + vxlan_id_(0), peer_identifier_(0), deleted_(false), vn_(NULL) { local_olist_.clear(); }; virtual ~MulticastGroupObject() { }; bool CanBeDeleted() const; - uint32_t evpn_mpls_label() const {return evpn_mpls_label_;} - void set_evpn_mpls_label(uint32_t label) {evpn_mpls_label_ = label;} //Add local member is local VM in server. bool AddLocalMember(const boost::uuids::uuid &intf_uuid) { @@ -116,7 +112,9 @@ class MulticastGroupObject { uint32_t vxlan_id() const {return vxlan_id_;} void set_peer_identifier(uint64_t peer_id) {peer_identifier_ = peer_id;} uint64_t peer_identifier() {return peer_identifier_;} - void CreateEvpnMplsLabel(const Agent *agent); + void set_vn(const VnEntry *vn); + void reset_vn(); + const VnEntry *vn() const {return vn_.get();} private: @@ -124,11 +122,11 @@ class MulticastGroupObject { Ip4Address grp_address_; std::string vn_name_; Ip4Address src_address_; - uint32_t evpn_mpls_label_; uint32_t vxlan_id_; uint64_t peer_identifier_; bool deleted_; std::list local_olist_; /* UUID of local i/f */ + VnEntryConstRef vn_; friend class MulticastHandler; DISALLOW_COPY_AND_ASSIGN(MulticastGroupObject); @@ -144,7 +142,7 @@ class MulticastHandler { MulticastGroupObject *CreateMulticastGroupObject(const string &vrf_name, const Ip4Address &ip_addr, - const string &vn_name, + const VnEntry *vn, uint32_t vxlan_id); /* Called by XMPP to add ctrl node sent olist and label */ diff --git a/src/vnsw/agent/oper/test/test_intf.cc b/src/vnsw/agent/oper/test/test_intf.cc index c00d62cb84c..a16254844af 100644 --- a/src/vnsw/agent/oper/test/test_intf.cc +++ b/src/vnsw/agent/oper/test/test_intf.cc @@ -1830,7 +1830,7 @@ TEST_F(IntfTest, VmPortServiceVlanAdd_2) { NovaDel(1); ConfigDel(1); client->WaitForIdle(); - EXPECT_TRUE(VmPortFindRetDel(1) == false); + EXPECT_TRUE(VmPortFind(1) == false); //Cleanup DelNode("virtual-machine-interface", input[0].name); @@ -1845,6 +1845,7 @@ TEST_F(IntfTest, VmPortServiceVlanAdd_2) { DelVn("vn2"); DeleteVmportEnv(input, 1, true); client->WaitForIdle(); + EXPECT_TRUE(VmPortFindRetDel(1) == false); EXPECT_FALSE(VrfFind("vrf1")); EXPECT_FALSE(VrfFind("vrf2")); } diff --git a/src/vnsw/agent/test/test_l2route.cc b/src/vnsw/agent/test/test_l2route.cc index ee8b48c57d2..c11d00b7a5e 100644 --- a/src/vnsw/agent/test/test_l2route.cc +++ b/src/vnsw/agent/test/test_l2route.cc @@ -879,15 +879,21 @@ TEST_F(RouteTest, evpn_mcast_label_deleted) { //Delete VM DeleteVmportEnv(input, 1, false); client->WaitForIdle(); - //Label should have been released - EXPECT_TRUE(agent_->mpls_table()->FindActiveEntry(new MplsLabelKey(evpn_mpls_label)) == NULL); + //Because VN is pending. + EXPECT_TRUE(agent_->mpls_table()->FindActiveEntry(new MplsLabelKey(evpn_mpls_label)) != NULL); rt = L2RouteGet("vrf1", MacAddress::FromString("ff:ff:ff:ff:ff:ff"), Ip4Address(0)); WAIT_FOR(1000, 1000, (rt->FindPath(agent_->local_vm_peer()) == NULL)); EXPECT_TRUE(rt != NULL); //Label is retained as evpn still present - EXPECT_TRUE(rt->GetActivePath()->label() == evpn_mpls_label); + EXPECT_TRUE(rt->GetActivePath()->label() == 0); + EXPECT_TRUE(rt->FindPath(agent_->local_peer())->label() == evpn_mpls_label); + EXPECT_TRUE(FindMplsLabel(MplsLabel::MCAST_NH, evpn_mpls_label)); + MplsLabel *mpls_entry = GetActiveLabel(MplsLabel::MCAST_NH, + evpn_mpls_label); + EXPECT_TRUE(mpls_entry->nexthop() == + rt->GetActiveNextHop()); //Delete EVPN TunnelOlist del_olist; @@ -903,6 +909,11 @@ TEST_F(RouteTest, evpn_mcast_label_deleted) { //route should get deleted EXPECT_TRUE(rt->FindPath(bgp_peer_ptr) == NULL); + //Label should have been released + DeleteVmportEnv(input, 1, true); + client->WaitForIdle(); + EXPECT_TRUE(agent_->mpls_table()->FindActiveEntry(new MplsLabelKey(evpn_mpls_label)) == NULL); + CreateVmportEnv(input, 1); client->WaitForIdle(); rt = L2RouteGet("vrf1", @@ -1083,8 +1094,8 @@ TEST_F(RouteTest, delete_notify_on_multicast_rt_with_no_state) { Composite::L2COMP); client->WaitForIdle(); DeleteBgpPeer(bgp_peer.get()); - client->WaitForIdle(); bgp_peer.reset(); + client->WaitForIdle(); } // Reset agentxmppchannel in active xmpp channel of agent. @@ -1128,10 +1139,10 @@ TEST_F(RouteTest, delpeer_walk_on_deleted_vrf) { client->WaitForIdle(); DeleteVmportEnv(input, 1, true); - client->WaitForIdle(); - DeleteBgpPeer(bgp_peer.get()); + DeleteBgpPeer(bgp_peer_ptr); client->WaitForIdle(); bgp_peer.reset(); + client->WaitForIdle(); } TEST_F(RouteTest, notify_walk_on_deleted_vrf_with_no_state_but_listener_id) { @@ -1174,8 +1185,8 @@ TEST_F(RouteTest, notify_walk_on_deleted_vrf_with_no_state_but_listener_id) { client->WaitForIdle(); //Release VRF reference vrf_ref.reset(); - client->WaitForIdle(); bgp_peer.reset(); + client->WaitForIdle(); } TEST_F(RouteTest, add_route_in_vrf_with_delayed_vn_vrf_link_add) { @@ -1369,10 +1380,10 @@ TEST_F(RouteTest, multiple_peer_evpn_label_check) { IpAddress::from_string("0.0.0.0").to_v4(), 4100, olist, 1); client->WaitForIdle(); - MulticastGroupObject *obj = - mc_handler->FindFloodGroupObject("vrf1"); - uint32_t evpn_label = obj->evpn_mpls_label(); - EXPECT_FALSE(FindMplsLabel(MplsLabel::MCAST_NH, evpn_label)); + //MulticastGroupObject *obj = + // mc_handler->FindFloodGroupObject("vrf1"); + //uint32_t evpn_label = obj->evpn_mpls_label(); + //EXPECT_FALSE(FindMplsLabel(MplsLabel::MCAST_NH, evpn_label)); //Delete remote paths mc_handler->ModifyFabricMembers(Agent::GetInstance()-> @@ -1496,6 +1507,44 @@ TEST_F(RouteTest, multicast_peer_with_stale_route) { client->WaitForIdle(); } +TEST_F(RouteTest, add_local_peer_and_then_vm) { + struct PortInfo input[] = { + {"vnet1", 1, "1.1.1.10", "00:00:00:01:01:01", 1, 1}, + }; + + client->Reset(); + CreateVmportEnv(input, 1); + client->WaitForIdle(); + + //Add a peer and enqueue path add in multicast route. + BgpPeer *bgp_peer_ptr = CreateBgpPeer(Ip4Address(1), "BGP Peer1"); + boost::shared_ptr bgp_peer = + bgp_peer_ptr->GetBgpXmppPeer()->bgp_peer_id_ref(); + client->WaitForIdle(); + BridgeRouteEntry *rt = L2RouteGet("vrf1", + MacAddress::FromString("ff:ff:ff:ff:ff:ff"), + Ip4Address(0)); + EXPECT_TRUE(rt->FindPath(agent_->multicast_peer())); + + MulticastGroupObject *obj = + MulticastHandler::GetInstance()->FindFloodGroupObject("vrf1"); + EXPECT_TRUE(obj->vn() != NULL); + DeleteVmportEnv(input, 1, false); + client->WaitForIdle(); + rt = L2RouteGet("vrf1", + MacAddress::FromString("ff:ff:ff:ff:ff:ff"), + Ip4Address(0)); + obj = + MulticastHandler::GetInstance()->FindFloodGroupObject("vrf1"); + EXPECT_TRUE(obj != NULL); + + DeleteVmportEnv(input, 1, true); + client->WaitForIdle(); + DeleteBgpPeer(bgp_peer.get()); + client->WaitForIdle(); + bgp_peer.reset(); +} + int main(int argc, char *argv[]) { ::testing::InitGoogleTest(&argc, argv); GETUSERARGS();