From f8f09e3f94590333a3f1a38acff47aeb834c1f18 Mon Sep 17 00:00:00 2001 From: Manish Date: Thu, 21 May 2015 14:51:40 +0530 Subject: [PATCH] Mac route in agent was having invalid label and destination NH. Problem: Two issues were seen - 1) Label was wrongly interpreted as MPLS though it was VXLAN and never got corrected. Sequence of events was that EVPN route was received before global vrouter config. Encapsulation priorities was absent in agent, so evpn route was programmed with Mpls-gre(control-node had sent Vxlan). Since Mpls encap was chosen, label was interpreted as mpls label and not vxlan. When global vrouter config was received resync was done for route. Resync also failed to rectify encap to Vxlan(since vxlan is now available in priorities) because this decision to rectify is based on vxlan id(i.e. if vxlan id is 0 default to mpls as its invalid). In this case vxlan id was 0 as explained above and hence encap continued to be Mpls. 2) Nexthop was different between evpn route and derived mac route. This happened because in path sync of evpn route return was false even though NH change was seen which resulted in avoidance of mac route rebake. Return was false because value set by ChangeNh as true was overridden by MplsChange. Solution: For case 1) - If encap is Vxlan only in the message sent by control-node then put label as vxlan id and mpls label as invalid, even though tunnel type is computed as Mpls(encapsulation prioirties is not received). In case of Mpls encap sent use label as Mpls and reset vxlan to invalid. In case both Vxlan and Mpls are sent in encap then fallback to old approach of interpreting label on computed tunnel type. For case 2) - Fix the return value. Change-Id: Ibeeb3de16d618ecb931c35d8937591d9c9f7f15e Closes-bug: 1457355 --- .../agent/controller/controller_route_path.cc | 34 ++++++++++++------ src/vnsw/agent/oper/agent_path.cc | 4 ++- src/vnsw/agent/test/test_l2route.cc | 35 +++++++++++++++++++ 3 files changed, 62 insertions(+), 11 deletions(-) diff --git a/src/vnsw/agent/controller/controller_route_path.cc b/src/vnsw/agent/controller/controller_route_path.cc index 779ff69b715..d34a46eee63 100644 --- a/src/vnsw/agent/controller/controller_route_path.cc +++ b/src/vnsw/agent/controller/controller_route_path.cc @@ -169,17 +169,31 @@ bool ControllerVmRoute::AddChangePath(Agent *agent, AgentPath *path, ret = true; } - if (new_tunnel_type == TunnelType::VXLAN) { - if (path->vxlan_id() != label_) { - path->set_vxlan_id(label_); - path->set_label(MplsTable::kInvalidLabel); - ret = true; - } + //Interpret label sent by control node + if (tunnel_bmap_ == TunnelType::VxlanType()) { + //Only VXLAN encap is sent, so label is VXLAN + path->set_vxlan_id(label_); + path->set_label(MplsTable::kInvalidLabel); + } else if (tunnel_bmap_ == TunnelType::MplsType()) { + //MPLS (GRE/UDP) is the only encap sent, + //so label is MPLS. + path->set_label(label_); + path->set_vxlan_id(VxLanTable::kInvalidvxlan_id); } else { - if (path->label() != label_) { - path->set_label(label_); - path->set_vxlan_id(VxLanTable::kInvalidvxlan_id); - ret = true; + //Got a mix of Vxlan and Mpls, so interpret label + //as per the computed tunnel type. + if (new_tunnel_type == TunnelType::VXLAN) { + if (path->vxlan_id() != label_) { + path->set_vxlan_id(label_); + path->set_label(MplsTable::kInvalidLabel); + ret = true; + } + } else { + if (path->label() != label_) { + path->set_label(label_); + path->set_vxlan_id(VxLanTable::kInvalidvxlan_id); + ret = true; + } } } diff --git a/src/vnsw/agent/oper/agent_path.cc b/src/vnsw/agent/oper/agent_path.cc index 12a42795ffc..251153caa35 100644 --- a/src/vnsw/agent/oper/agent_path.cc +++ b/src/vnsw/agent/oper/agent_path.cc @@ -100,7 +100,8 @@ bool AgentPath::ChangeNH(Agent *agent, NextHop *nh) { MplsLabelKey key(MplsLabel::MCAST_NH, label_); MplsLabel *mpls = static_cast(agent->mpls_table()-> FindActiveEntry(&key)); - ret = agent->mpls_table()->ChangeNH(mpls, nh); + if (agent->mpls_table()->ChangeNH(mpls, nh)) + ret = true; if (mpls) { //Send notify of change mpls->get_table_partition()->Notify(mpls); @@ -360,6 +361,7 @@ bool EvpnDerivedPathData::AddChangePath(Agent *agent, AgentPath *path, EvpnDerivedPath *evpn_path = dynamic_cast(path); assert(evpn_path != NULL); + evpn_path->set_server_ip(reference_path_->server_ip()); uint32_t label = reference_path_->label(); if (evpn_path->label() != label) { evpn_path->set_label(label); diff --git a/src/vnsw/agent/test/test_l2route.cc b/src/vnsw/agent/test/test_l2route.cc index 05e86739d10..27fa0410dbf 100644 --- a/src/vnsw/agent/test/test_l2route.cc +++ b/src/vnsw/agent/test/test_l2route.cc @@ -615,6 +615,41 @@ TEST_F(RouteTest, vxlan_network_id_change_for_non_l2_interface) { client->WaitForIdle(); } +TEST_F(RouteTest, RemoteVxlanEncapRoute_1) { + client->Reset(); + VxLanNetworkIdentifierMode(false); + client->WaitForIdle(); + RouteTest::SetTunnelType(TunnelType::MPLS_GRE); + DelEncapList(); + client->WaitForIdle(); + + TunnelType::TypeBmap bmap = TunnelType::VxlanType(); + AddRemoteVmRoute(remote_vm_mac_, remote_vm_ip4_, server1_ip_, + MplsTable::kStartLabel, bmap); + WAIT_FOR(1000, 100, + (L2RouteFind(vrf_name_, remote_vm_mac_, remote_vm_ip4_) == true)); + + BridgeRouteEntry *rt = L2RouteGet(vrf_name_, remote_vm_mac_, + remote_vm_ip4_); + const AgentPath *path = rt->GetActivePath(); + EXPECT_TRUE(path->tunnel_type() != TunnelType::VXLAN); + + //Update VXLAN in encap prio + AddEncapList("MPLSoGRE", "MPLSoUDP", "VXLAN"); + client->WaitForIdle(); + path = rt->GetActivePath(); + EXPECT_TRUE(path->tunnel_type() == TunnelType::VXLAN); + const TunnelNH *nh = + static_cast(path->nexthop()); + EXPECT_TRUE(nh->GetDip()->to_string() == server1_ip_.to_string()); + + DeleteRoute(agent_->local_peer(), vrf_name_, remote_vm_mac_, + remote_vm_ip4_); + client->WaitForIdle(); + DelEncapList(); + client->WaitForIdle(); +} + TEST_F(RouteTest, Enqueue_l2_route_add_on_deleted_vrf) { struct PortInfo input[] = { {"vnet1", 1, "1.1.1.10", "00:00:00:01:01:01", 1, 1},