From 795fde888eb5edda828432d5c0a774f794be709e Mon Sep 17 00:00:00 2001 From: Naveen N Date: Thu, 1 Dec 2016 16:17:07 +0530 Subject: [PATCH] * Try ARP request even when EVPN route is in wait for traffic mode Currently if only EVPN route is in backup mode, GARP packets are not trapped to agent, hence upon mastership switch agent would not be able to identify and update EVPN route preference. Its easily reproducible by changing the group id of VRRP. Consider VM 1 was configured with group ID 100, but AAP was configured with AAP vmac of group ID 10, now prefernece of IP route would be bumped but not EVPN route since mac of GARP packets doesnt match with AAP configured mac. Now if group ID is changed to 10, VRRP would still send a GARP but it would not be trapped to agent, since l3 route is already in high prefernece. Fixing the same by having TRAP flag set even if EVPN route is in backup mode, also from flow if EVPN route is in low preference. Closes-bug:#1644170 Change-Id: I7e9e87f32c453159a56d828bfb16a1dca8627623 --- src/vnsw/agent/pkt/flow_table.cc | 10 + src/vnsw/agent/pkt/flow_table.h | 2 + src/vnsw/agent/pkt/pkt_flow_info.cc | 61 +++-- src/vnsw/agent/pkt/pkt_flow_info.h | 3 + src/vnsw/agent/pkt/test/test_pkt_flow.cc | 30 +++ src/vnsw/agent/services/arp_proto.cc | 252 ++++++++++++++---- src/vnsw/agent/services/arp_proto.h | 136 +++++++++- src/vnsw/agent/services/test/SConscript | 3 +- .../services/test/arp_path_preference_test.cc | 163 +++++++++++ src/vnsw/agent/vrouter/ksync/route_ksync.cc | 44 ++- src/vnsw/agent/vrouter/ksync/route_ksync.h | 34 ++- .../vrouter/ksync/test/test_ksync_route.cc | 29 ++ 12 files changed, 684 insertions(+), 83 deletions(-) create mode 100644 src/vnsw/agent/services/test/arp_path_preference_test.cc diff --git a/src/vnsw/agent/pkt/flow_table.cc b/src/vnsw/agent/pkt/flow_table.cc index 6c38b3d7b66..771c65e13fc 100644 --- a/src/vnsw/agent/pkt/flow_table.cc +++ b/src/vnsw/agent/pkt/flow_table.cc @@ -3369,6 +3369,16 @@ AgentRoute *FlowTable::GetL2Route(const VrfEntry *vrf, return table->FindRoute(mac); } +//Find EVPN route +AgentRoute* FlowTable::GetEvpnRoute(const VrfEntry *vrf, + const MacAddress &mac, + const IpAddress &ip, + uint32_t ethernet_tag) { + EvpnAgentRouteTable *table = static_cast( + vrf->GetEvpnRouteTable()); + return table->FindRoute(mac, ip, ethernet_tag); +} + AgentRoute *FlowTable::GetUcRoute(const VrfEntry *entry, const IpAddress &addr) { AgentRoute *rt = NULL; diff --git a/src/vnsw/agent/pkt/flow_table.h b/src/vnsw/agent/pkt/flow_table.h index a555ca8a8bc..642884443f2 100644 --- a/src/vnsw/agent/pkt/flow_table.h +++ b/src/vnsw/agent/pkt/flow_table.h @@ -720,6 +720,8 @@ class FlowTable { DBTableBase::ListenerId nh_listener_id(); AgentRoute *GetL2Route(const VrfEntry *entry, const MacAddress &mac); + AgentRoute *GetEvpnRoute(const VrfEntry *entry, const MacAddress &mac, + const IpAddress &addr, uint32_t ethernet_tag); AgentRoute *GetUcRoute(const VrfEntry *entry, const IpAddress &addr); static const SecurityGroupList &default_sg_list() {return default_sg_list_;} bool ValidFlowMove(const FlowEntry *new_flow, diff --git a/src/vnsw/agent/pkt/pkt_flow_info.cc b/src/vnsw/agent/pkt/pkt_flow_info.cc index 7334924c2b9..427cd506837 100644 --- a/src/vnsw/agent/pkt/pkt_flow_info.cc +++ b/src/vnsw/agent/pkt/pkt_flow_info.cc @@ -1353,6 +1353,48 @@ bool PktFlowInfo::Process(const PktInfo *pkt, PktControlInfo *in, return true; } +void PktFlowInfo::EnqueueTrafficSeen(const PktInfo *pkt, + PktControlInfo *in, + PktControlInfo *out) { + if (pkt->family != Address::INET) { + return; + } + + Ip4Address v4_src = pkt->ip_saddr.to_v4(); + if (short_flow || linklocal_flow || !ingress) { + return; + } + + const AgentRoute *rt = NULL; + bool enqueue_traffic_seen = false; + const VmInterface *vm_intf = dynamic_cast(in->intf_); + + if (l3_flow) { + rt = in->rt_; + } else if (in->vrf_) { + rt = flow_table->GetUcRoute(in->vrf_, v4_src); + } + + if (rt && rt->WaitForTraffic()) { + enqueue_traffic_seen = true; + } else if (vm_intf) { + //L3 route is not in wait for traffic state + //EVPN route could be in wait for traffic, if yes + //enqueue traffic seen + rt = flow_table->GetEvpnRoute(in->vrf_, pkt->smac, v4_src, + vm_intf->ethernet_tag()); + if (rt && rt->WaitForTraffic()) { + enqueue_traffic_seen = true; + } + } + + if (enqueue_traffic_seen) { + flow_table->agent()->oper_db()->route_preference_module()-> + EnqueueTrafficSeen(v4_src, 32, in->intf_->id(), + pkt->vrf, pkt->smac); + } +} + void PktFlowInfo::Add(const PktInfo *pkt, PktControlInfo *in, PktControlInfo *out) { FlowKey key(in->nh_, pkt->ip_saddr, pkt->ip_daddr, pkt->ip_proto, @@ -1365,24 +1407,7 @@ void PktFlowInfo::Add(const PktInfo *pkt, PktControlInfo *in, Agent::GetInstance()->pkt()->flow_table()->DeleteFlowInfo(flow.get()); } - if (pkt->family == Address::INET) { - Ip4Address v4_src = pkt->ip_saddr.to_v4(); - if (ingress && !short_flow && !linklocal_flow) { - const AgentRoute *rt = NULL; - if (l3_flow) { - rt = in->rt_; - } else if (in->vrf_) { - rt = flow_table->GetUcRoute(in->vrf_, v4_src); - } - if (rt && rt->WaitForTraffic()) { - flow_table->agent()->oper_db()->route_preference_module()-> - EnqueueTrafficSeen(v4_src, 32, in->intf_->id(), - pkt->vrf, pkt->smac); - } - } - } else if (pkt->family == Address::INET6) { - //TODO:: Handle Ipv6 changes - } + EnqueueTrafficSeen(pkt, in, out); // Do not allow more than max flows if ((in->vm_ && diff --git a/src/vnsw/agent/pkt/pkt_flow_info.h b/src/vnsw/agent/pkt/pkt_flow_info.h index 284da953a96..059d83c76ba 100644 --- a/src/vnsw/agent/pkt/pkt_flow_info.h +++ b/src/vnsw/agent/pkt/pkt_flow_info.h @@ -96,6 +96,9 @@ class PktFlowInfo { const IpAddress &addr, const MacAddress &mac, FlowRouteRefMap &ref_map); uint8_t RouteToPrefixLen(const AgentRoute *route); + void EnqueueTrafficSeen(const PktInfo *pkt, + PktControlInfo *in, + PktControlInfo *out); bool l3_flow; Address::Family family; diff --git a/src/vnsw/agent/pkt/test/test_pkt_flow.cc b/src/vnsw/agent/pkt/test/test_pkt_flow.cc index 390b6bc11a3..47e49586b94 100644 --- a/src/vnsw/agent/pkt/test/test_pkt_flow.cc +++ b/src/vnsw/agent/pkt/test/test_pkt_flow.cc @@ -591,6 +591,7 @@ class FlowTest : public ::testing::Test { }; bool FlowTest::ksync_init_; + //Ingress flow test (VMport to VMport - Same VN) //Flow creation using IP and TCP packets TEST_F(FlowTest, FlowAdd_1) { @@ -3853,6 +3854,35 @@ TEST_F(FlowTest, AclRuleUpdate) { client->WaitForIdle(5); } +//Create a layer2 flow and verify that we dont add layer2 route +//in prefix length manipulation +TEST_F(FlowTest, WaitForTraffic) { + Ip4Address ip = Ip4Address::from_string(vm1_ip); + MacAddress mac(0, 0, 0, 1, 1, 1); + + AgentRoute *ip_rt = RouteGet("vrf5", Ip4Address::from_string(vm1_ip), 32); + AgentRoute *evpn_Rt = EvpnRouteGet("vrf5", mac, ip, 0); + + EXPECT_TRUE(ip_rt->WaitForTraffic() == true); + EXPECT_TRUE(evpn_Rt->WaitForTraffic() == true); + + //Enqueue a flow with wrong mac address + TxL2Packet(flow0->id(),input[2].mac, input[1].mac, + input[0].addr, input[1].addr, 1); + client->WaitForIdle(); + //Only IP route should goto wait for traffic + EXPECT_TRUE(ip_rt->WaitForTraffic() == false); + EXPECT_TRUE(evpn_Rt->WaitForTraffic() == true); + + //Enqueue flow with right mac address + //EVPN route should also goto traffic seen state + TxL2Packet(flow0->id(),input[0].mac, input[1].mac, + input[0].addr, input[1].addr, 1); + client->WaitForIdle(); + EXPECT_TRUE(ip_rt->WaitForTraffic() == false); + EXPECT_TRUE(evpn_Rt->WaitForTraffic() == false); +} + int main(int argc, char *argv[]) { GETUSERARGS(); diff --git a/src/vnsw/agent/services/arp_proto.cc b/src/vnsw/agent/services/arp_proto.cc index 8853c5bb5fe..fbdff65e7b9 100644 --- a/src/vnsw/agent/services/arp_proto.cc +++ b/src/vnsw/agent/services/arp_proto.cc @@ -73,32 +73,64 @@ void ArpProto::VrfNotify(DBTablePartBase *part, DBEntryBase *entry) { if (!state){ state = new ArpVrfState(agent_, this, vrf, - vrf->GetInet4UnicastRouteTable()); + vrf->GetInet4UnicastRouteTable(), + vrf->GetEvpnRouteTable()); state->route_table_listener_id = vrf-> GetInet4UnicastRouteTable()-> Register(boost::bind(&ArpVrfState::RouteUpdate, state, _1, _2)); + state->evpn_route_table_listener_id = vrf->GetEvpnRouteTable()-> + Register(boost::bind(&ArpVrfState::EvpnRouteUpdate, state, _1, _2)); entry->SetState(part->parent(), vrf_table_listener_id_, state); } } -ArpDBState::ArpDBState(ArpVrfState *vrf_state, uint32_t vrf_id, IpAddress ip, - uint8_t plen) : vrf_state_(vrf_state), - arp_req_timer_(NULL), vrf_id_(vrf_id), vm_ip_(ip), plen_(plen), - sg_list_(0), policy_(false), resolve_route_(false) { +void intrusive_ptr_add_ref(ArpPathPreferenceState *aps) { + aps->refcount_.fetch_and_increment(); } -ArpDBState::~ArpDBState() { +void intrusive_ptr_release(ArpPathPreferenceState *aps) { + ArpVrfState *state = aps->vrf_state(); + int prev = aps->refcount_.fetch_and_decrement(); + if (prev == 1) { + state->Erase(aps->ip()); + delete aps; + } +} + +ArpPathPreferenceState::ArpPathPreferenceState(ArpVrfState *state, + uint32_t vrf_id, + const IpAddress &ip, + uint8_t plen): + vrf_state_(state), arp_req_timer_(NULL), vrf_id_(vrf_id), + vm_ip_(ip), plen_(plen), gw_ip_(Ip4Address(0)) { + refcount_ = 0; +} + +ArpPathPreferenceState::~ArpPathPreferenceState() { if (arp_req_timer_) { arp_req_timer_->Cancel(); TimerManager::DeleteTimer(arp_req_timer_); } + assert(refcount_ == 0); } -bool ArpDBState::SendArpRequest() { - if (wait_for_traffic_map_.size() == 0) { - return false; +void ArpPathPreferenceState::StartTimer() { + if (arp_req_timer_ == NULL) { + arp_req_timer_ = TimerManager::CreateTimer( + *(vrf_state_->agent->event_manager()->io_service()), + "Arp Entry timer for VM", + TaskScheduler::GetInstance()-> + GetTaskId("Agent::Services"), PktHandler::ARP); } + arp_req_timer_->Start(kTimeout, + boost::bind(&ArpPathPreferenceState::SendArpRequest, + this)); +} +bool ArpPathPreferenceState::SendArpRequest(WaitForTrafficIntfMap + &wait_for_traffic_map, + ArpTransmittedIntfMap + &arp_transmitted_map) { bool ret = false; boost::shared_ptr pkt(new PktInfo(vrf_state_->agent, ARP_TX_BUFF_LEN, @@ -106,8 +138,8 @@ bool ArpDBState::SendArpRequest() { ArpHandler arp_handler(vrf_state_->agent, pkt, *(vrf_state_->agent->event_manager()->io_service())); - WaitForTrafficIntfMap::iterator it = wait_for_traffic_map_.begin(); - for (;it != wait_for_traffic_map_.end(); it++) { + WaitForTrafficIntfMap::iterator it = wait_for_traffic_map.begin(); + for (;it != wait_for_traffic_map.end(); it++) { if (it->second >= kMaxRetry) { continue; } @@ -118,7 +150,12 @@ bool ArpDBState::SendArpRequest() { continue; } MacAddress smac = vm_intf->GetVifMac(vrf_state_->agent); + bool inserted = arp_transmitted_map.insert(it->first).second; it->second++; + if (inserted == false) { + //ARP request already sent due to IP route + continue; + } arp_handler.SendArp(ARPOP_REQUEST, smac, gw_ip_.to_v4().to_ulong(), MacAddress(), vm_ip_.to_v4().to_ulong(), @@ -129,22 +166,34 @@ bool ArpDBState::SendArpRequest() { return ret; } -void ArpDBState::StartTimer() { - if (arp_req_timer_ == NULL) { - arp_req_timer_ = TimerManager::CreateTimer( - *(vrf_state_->agent->event_manager()->io_service()), - "Arp Entry timer for VM", - TaskScheduler::GetInstance()-> - GetTaskId("Agent::Services"), PktHandler::ARP); +bool ArpPathPreferenceState::SendArpRequest() { + if (l3_wait_for_traffic_map_.size() == 0 && + evpn_wait_for_traffic_map_.size() == 0) { + return false; } - arp_req_timer_->Start(kTimeout, boost::bind(&ArpDBState::SendArpRequest, - this)); + + bool ret = false; + ArpTransmittedIntfMap arp_transmitted_map; + if (SendArpRequest(l3_wait_for_traffic_map_, arp_transmitted_map)) { + ret = true; + } + + if (SendArpRequest(evpn_wait_for_traffic_map_, arp_transmitted_map)) { + ret = true; + } + + return ret; } //Send ARP request on interface in Active-BackUp mode //So that preference of route can be incremented if the VM replies to ARP -void ArpDBState::SendArpRequestForAllIntf(const InetUnicastRouteEntry *route) { +void ArpPathPreferenceState::SendArpRequestForAllIntf(const AgentRoute *route) { WaitForTrafficIntfMap new_wait_for_traffic_map; + WaitForTrafficIntfMap &wait_for_traffic_map = evpn_wait_for_traffic_map_; + if (dynamic_cast(route)) { + wait_for_traffic_map = l3_wait_for_traffic_map_; + } + for (Route::PathList::const_iterator it = route->GetPathList().begin(); it != route->GetPathList().end(); it++) { const AgentPath *path = static_cast(it.operator->()); @@ -166,14 +215,19 @@ void ArpDBState::SendArpRequestForAllIntf(const InetUnicastRouteEntry *route) { if (path->subnet_service_ip().is_v4() == false) { continue; } - gw_ip_ = path->subnet_service_ip(); + + //Pick service IP from IP route, + //we could always send ARP probe with zero IP?? + if (dynamic_cast(route)) { + gw_ip_ = path->subnet_service_ip(); + } uint32_t intf_id = intf->id(); bool wait_for_traffic = path->path_preference().wait_for_traffic(); //Build new list of interfaces in active state if (wait_for_traffic == true) { WaitForTrafficIntfMap::const_iterator wait_for_traffic_it = - wait_for_traffic_map_.find(intf_id); - if (wait_for_traffic_it == wait_for_traffic_map_.end()) { + wait_for_traffic_map.find(intf_id); + if (wait_for_traffic_it == wait_for_traffic_map.end()) { new_wait_for_traffic_map.insert(std::make_pair(intf_id, 0)); } else { new_wait_for_traffic_map.insert(std::make_pair(intf_id, @@ -183,14 +237,28 @@ void ArpDBState::SendArpRequestForAllIntf(const InetUnicastRouteEntry *route) { } } - - wait_for_traffic_map_ = new_wait_for_traffic_map; - if (wait_for_traffic_map_.size() > 0) { + if (dynamic_cast(route)) { + l3_wait_for_traffic_map_ = new_wait_for_traffic_map; + } else { + evpn_wait_for_traffic_map_ = new_wait_for_traffic_map; + } + if (new_wait_for_traffic_map.size() > 0) { SendArpRequest(); StartTimer(); } } +ArpDBState::ArpDBState(ArpVrfState *vrf_state, uint32_t vrf_id, IpAddress ip, + uint8_t plen) : vrf_state_(vrf_state), + sg_list_(0), policy_(false), resolve_route_(false) { + if (plen == Address::kMaxV4PrefixLen && ip != Ip4Address(0)) { + arp_path_preference_state_.reset(vrf_state->Locate(ip)); + } +} + +ArpDBState::~ArpDBState() { +} + void ArpDBState::UpdateArpRoutes(const InetUnicastRouteEntry *rt) { int plen = rt->plen(); uint32_t start_ip = rt->addr().to_v4().to_ulong(); @@ -229,26 +297,59 @@ void ArpDBState::Delete(const InetUnicastRouteEntry *rt) { } } -void ArpDBState::Update(const InetUnicastRouteEntry *rt) { +void ArpDBState::Update(const AgentRoute *rt) { + if (arp_path_preference_state_) { + arp_path_preference_state_->SendArpRequestForAllIntf(rt); + } + + const InetUnicastRouteEntry *ip_rt = + dynamic_cast(rt); + if (ip_rt == NULL) { + return; + } + if (rt->GetActiveNextHop()->GetType() == NextHop::RESOLVE) { resolve_route_ = true; } - bool policy = rt->GetActiveNextHop()->PolicyEnabled(); - const SecurityGroupList sg = rt->GetActivePath()->sg_list(); + bool policy = ip_rt->GetActiveNextHop()->PolicyEnabled(); + const SecurityGroupList sg = ip_rt->GetActivePath()->sg_list(); if (policy_ != policy || sg != sg_list_ || - vn_ != rt->GetActivePath()->dest_vn_name()) { + vn_ != ip_rt->GetActivePath()->dest_vn_name()) { policy_ = policy; sg_list_ = sg; - vn_ = rt->GetActivePath()->dest_vn_name(); + vn_ = ip_rt->GetActivePath()->dest_vn_name(); if (resolve_route_) { - UpdateArpRoutes(rt); + UpdateArpRoutes(ip_rt); } } } +void ArpVrfState::EvpnRouteUpdate(DBTablePartBase *part, DBEntryBase *entry) { + EvpnRouteEntry *route = static_cast(entry); + + ArpDBState *state = static_cast(entry->GetState(part->parent(), + evpn_route_table_listener_id)); + + if (entry->IsDeleted() || deleted) { + if (state) { + entry->ClearState(part->parent(), evpn_route_table_listener_id); + delete state; + } + return; + } + + if (state == NULL) { + state = new ArpDBState(this, route->vrf_id(), route->ip_addr(), + route->GetVmIpPlen()); + entry->SetState(part->parent(), evpn_route_table_listener_id, state); + } + + state->Update(route); +} + void ArpVrfState::RouteUpdate(DBTablePartBase *part, DBEntryBase *entry) { InetUnicastRouteEntry *route = static_cast(entry); @@ -289,7 +390,6 @@ void ArpVrfState::RouteUpdate(DBTablePartBase *part, DBEntryBase *entry) { //ARP request, to trigger route preference state machine if (state && route->vrf()->GetName() != agent->fabric_vrf_name()) { state->Update(route); - state->SendArpRequestForAllIntf(route); } } @@ -298,28 +398,75 @@ bool ArpVrfState::DeleteRouteState(DBTablePartBase *part, DBEntryBase *entry) { return true; } +bool ArpVrfState::DeleteEvpnRouteState(DBTablePartBase *part, DBEntryBase *entry) { + EvpnRouteUpdate(part, entry); + return true; +} + void ArpVrfState::Delete() { deleted = true; DBTableWalker *walker = agent->db()->GetWalker(); - walker->WalkTable(rt_table, NULL, - boost::bind(&ArpVrfState::DeleteRouteState, this, _1, _2), - boost::bind(&ArpVrfState::WalkDone, this, _1, this)); + walk_id_ = walker->WalkTable(rt_table, NULL, + boost::bind(&ArpVrfState::DeleteRouteState, this, _1, _2), + boost::bind(&ArpVrfState::WalkDone, _1, this)); + evpn_walk_id_ = walker->WalkTable(evpn_rt_table, NULL, + boost::bind(&ArpVrfState::DeleteEvpnRouteState, this, _1, _2), + boost::bind(&ArpVrfState::WalkDone, _1, this)); +} + +bool ArpVrfState::PreWalkDone(DBTableBase *partition) { + if (arp_proto->ValidateAndClearVrfState(vrf, this) == false) { + return false; + } + + rt_table->Unregister(route_table_listener_id); + table_delete_ref.Reset(NULL); + + evpn_rt_table->Unregister(evpn_route_table_listener_id); + evpn_table_delete_ref.Reset(NULL); + return true; } void ArpVrfState::WalkDone(DBTableBase *partition, ArpVrfState *state) { - if (arp_proto->ValidateAndClearVrfState(vrf, state) == false) - return; + if (partition == state->rt_table) { + state->walk_id_ = DBTableWalker::kInvalidWalkerId; + state->l3_walk_completed_ = true; + } else { + state->evpn_walk_id_ = DBTableWalker::kInvalidWalkerId; + state->evpn_walk_completed_ = true; + } + + if (state->PreWalkDone(partition)) { + delete state; + } +} + +ArpPathPreferenceState* ArpVrfState::Locate(const IpAddress &ip) { + ArpPathPreferenceState* ptr = arp_path_preference_map_[ip]; + + if (ptr == NULL) { + ptr = new ArpPathPreferenceState(this, vrf->vrf_id(), ip, 32); + arp_path_preference_map_[ip] = ptr; + } + return ptr; +} - state->rt_table->Unregister(route_table_listener_id); - state->table_delete_ref.Reset(NULL); - delete state; +void ArpVrfState::Erase(const IpAddress &ip) { + arp_path_preference_map_.erase(ip); } ArpVrfState::ArpVrfState(Agent *agent_ptr, ArpProto *proto, VrfEntry *vrf_entry, - AgentRouteTable *table): + AgentRouteTable *table, AgentRouteTable *evpn_table): agent(agent_ptr), arp_proto(proto), vrf(vrf_entry), rt_table(table), - route_table_listener_id(DBTableBase::kInvalidId), - table_delete_ref(this, table->deleter()), deleted(false) { + evpn_rt_table(evpn_table), route_table_listener_id(DBTableBase::kInvalidId), + evpn_route_table_listener_id(DBTableBase::kInvalidId), + table_delete_ref(this, table->deleter()), + evpn_table_delete_ref(this, evpn_table->deleter()), + deleted(false), l3_walk_completed_(false), evpn_walk_completed_(false) { +} + +ArpVrfState::~ArpVrfState() { + assert(arp_path_preference_map_.size() == 0); } void ArpProto::InterfaceNotify(DBEntryBase *entry) { @@ -526,6 +673,21 @@ bool ArpProto::ValidateAndClearVrfState(VrfEntry *vrf, return false; } + if (vrf_state->l3_walk_completed() == false) { + return false; + } + + if (vrf_state->evpn_walk_completed() == false) { + return false; + } + + if (vrf_state->walk_id_ != DBTableWalker::kInvalidWalkerId || + vrf_state->evpn_walk_id_ != DBTableWalker::kInvalidWalkerId) { + ARP_TRACE(Trace, "ARP state not cleared - Route table walk not complete", + "", vrf->GetName(), ""); + return false; + } + DBState *state = static_cast (vrf->GetState(vrf->get_table_partition()->parent(), vrf_table_listener_id_)); diff --git a/src/vnsw/agent/services/arp_proto.h b/src/vnsw/agent/services/arp_proto.h index f2100b10f82..a68b4720688 100644 --- a/src/vnsw/agent/services/arp_proto.h +++ b/src/vnsw/agent/services/arp_proto.h @@ -160,6 +160,9 @@ class ArpProto : public Proto { ArpIterator FindUpperBoundArpEntry(const ArpKey &key); ArpIterator FindLowerBoundArpEntry(const ArpKey &key); + DBTableBase::ListenerId vrf_table_listener_id() const { + return vrf_table_listener_id_; + } private: void VrfNotify(DBTablePartBase *part, DBEntryBase *entry); void NextHopNotify(DBEntryBase *entry); @@ -189,24 +192,141 @@ class ArpProto : public Proto { DISALLOW_COPY_AND_ASSIGN(ArpProto); }; +//Stucture used to retry ARP queries when a particular route is in +//backup state. +class ArpPathPreferenceState { +public: + static const uint32_t kMaxRetry = 30 * 5; //retries upto 5 minutes, + //30 tries/per minutes + static const uint32_t kTimeout = 2000; + typedef std::map WaitForTrafficIntfMap; + typedef std::set ArpTransmittedIntfMap; + + ArpPathPreferenceState(ArpVrfState *state, uint32_t vrf_id, + const IpAddress &vm_ip, uint8_t plen); + ~ArpPathPreferenceState(); + + bool SendArpRequest(); + bool SendArpRequest(WaitForTrafficIntfMap &wait_for_traffic_map, + ArpTransmittedIntfMap &arp_transmitted_intf_map); + void SendArpRequestForAllIntf(const AgentRoute *route); + void StartTimer(); + + ArpVrfState* vrf_state() { + return vrf_state_; + } + + const IpAddress& ip() const { + return vm_ip_; + } + + bool IntfPresentInIpMap(uint32_t id) const { + if (l3_wait_for_traffic_map_.find(id) == + l3_wait_for_traffic_map_.end()) { + return false; + } + return true; + } + + bool IntfPresentInEvpnMap(uint32_t id) const { + if (evpn_wait_for_traffic_map_.find(id) == + evpn_wait_for_traffic_map_.end()) { + return false; + } + return true; + } + + uint32_t IntfRetryCountInIpMap(uint32_t id) const { + WaitForTrafficIntfMap::const_iterator it = + l3_wait_for_traffic_map_.find(id); + if (it == l3_wait_for_traffic_map_.end()) { + return 0; + } + return it->second; + } + + uint32_t IntfRetryCountInEvpnMap(uint32_t id) const { + WaitForTrafficIntfMap::const_iterator it = + evpn_wait_for_traffic_map_.find(id); + if (it == evpn_wait_for_traffic_map_.end()) { + return 0; + } + return it->second; + } + +private: + friend void intrusive_ptr_add_ref(ArpPathPreferenceState *aps); + friend void intrusive_ptr_release(ArpPathPreferenceState *aps); + ArpVrfState *vrf_state_; + Timer *arp_req_timer_; + uint32_t vrf_id_; + IpAddress vm_ip_; + uint8_t plen_; + IpAddress gw_ip_; + WaitForTrafficIntfMap l3_wait_for_traffic_map_; + WaitForTrafficIntfMap evpn_wait_for_traffic_map_; + tbb::atomic refcount_; +}; + +typedef boost::intrusive_ptr ArpPathPreferenceStatePtr; + +void intrusive_ptr_add_ref(ArpPathPreferenceState *aps); +void intrusive_ptr_release(ArpPathPreferenceState *aps); + struct ArpVrfState : public DBState { public: + typedef std::map ArpPathPreferenceStateMap; + typedef std::pair ArpPathPreferenceStatePair; + ArpVrfState(Agent *agent, ArpProto *proto, VrfEntry *vrf, - AgentRouteTable *table); + AgentRouteTable *table, AgentRouteTable *evpn_table); + ~ArpVrfState(); void RouteUpdate(DBTablePartBase *part, DBEntryBase *entry); + void EvpnRouteUpdate(DBTablePartBase *part, DBEntryBase *entry); void ManagedDelete() { deleted = true;} void SendArpRequestForVm(InetUnicastRouteEntry *route); void Delete(); bool DeleteRouteState(DBTablePartBase *part, DBEntryBase *entry); - void WalkDone(DBTableBase *partition, ArpVrfState *state); + bool DeleteEvpnRouteState(DBTablePartBase *part, DBEntryBase *entry); + static void WalkDone(DBTableBase *partition, ArpVrfState *state); + bool PreWalkDone(DBTableBase *partition); + ArpPathPreferenceState* Locate(const IpAddress &ip); + void Erase(const IpAddress &ip); + + const ArpPathPreferenceState* Get(const IpAddress ip) const { + ArpPathPreferenceStateMap::const_iterator it = + arp_path_preference_map_.find(ip); + if (it == arp_path_preference_map_.end()) { + return NULL; + } + return it->second; + } + + bool l3_walk_completed() const { + return l3_walk_completed_; + } + + bool evpn_walk_completed() const { + return evpn_walk_completed_; + } Agent *agent; ArpProto *arp_proto; VrfEntry *vrf; AgentRouteTable *rt_table; + AgentRouteTable *evpn_rt_table; DBTableBase::ListenerId route_table_listener_id; + DBTableBase::ListenerId evpn_route_table_listener_id; LifetimeRef table_delete_ref; + LifetimeRef evpn_table_delete_ref; bool deleted; + DBTableWalker::WalkId walk_id_; + DBTableWalker::WalkId evpn_walk_id_; + ArpPathPreferenceStateMap arp_path_preference_map_; + bool l3_walk_completed_; + bool evpn_walk_completed_; friend class ArpProto; }; @@ -219,23 +339,15 @@ class ArpDBState : public DBState { ArpDBState(ArpVrfState *vrf_state, uint32_t vrf_id, IpAddress vm_ip_addr, uint8_t plen); ~ArpDBState(); - bool SendArpRequest(); - void SendArpRequestForAllIntf(const InetUnicastRouteEntry *route); - void StartTimer(); - void Update(const InetUnicastRouteEntry *route); + void Update(const AgentRoute *route); void UpdateArpRoutes(const InetUnicastRouteEntry *route); void Delete(const InetUnicastRouteEntry *rt); private: ArpVrfState *vrf_state_; - Timer *arp_req_timer_; - uint32_t vrf_id_; - IpAddress vm_ip_; - uint8_t plen_; - IpAddress gw_ip_; SecurityGroupList sg_list_; bool policy_; bool resolve_route_; std::string vn_; - WaitForTrafficIntfMap wait_for_traffic_map_; + ArpPathPreferenceStatePtr arp_path_preference_state_; }; #endif // vnsw_agent_arp_proto_hpp diff --git a/src/vnsw/agent/services/test/SConscript b/src/vnsw/agent/services/test/SConscript index 792977d6da3..db6d857bec6 100644 --- a/src/vnsw/agent/services/test/SConscript +++ b/src/vnsw/agent/services/test/SConscript @@ -21,7 +21,8 @@ icmp_test = AgentEnv.MakeTestCmd(env, 'icmp_test', service_test_suite) icmpv6_test = AgentEnv.MakeTestCmd(env, 'icmpv6_test', service_flaky_test_suite) metadata_test = AgentEnv.MakeTestCmd(env, 'metadata_test', service_test_suite) pkt_trace_test = AgentEnv.MakeTestCmd(env, 'pkt_trace_test', service_flaky_test_suite) - +arp_path_preference_test = AgentEnv.MakeTestCmd(env, 'arp_path_preference_test', + service_test_suite) flaky_test = env.TestSuite('agent-flaky-test', service_flaky_test_suite) env.Alias('controller/src/vnsw/agent/services:flaky_test', flaky_test) diff --git a/src/vnsw/agent/services/test/arp_path_preference_test.cc b/src/vnsw/agent/services/test/arp_path_preference_test.cc new file mode 100644 index 00000000000..005a2c6a26e --- /dev/null +++ b/src/vnsw/agent/services/test/arp_path_preference_test.cc @@ -0,0 +1,163 @@ +/* + * Copyright (c) 2013 Juniper Networks, Inc. All rights reserved. + */ + +#include "base/os.h" +#include "testing/gunit.h" + +#include +#include +#include + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include "vr_types.h" +#include "xmpp/test/xmpp_test_util.h" +#include +#include "oper/path_preference.h" + +struct PortInfo input1[] = { + {"vnet1", 1, "8.1.1.1", "00:00:00:01:01:01", 1, 1} +}; + +struct PortInfo input2[] = { + {"vnet2", 2, "8.1.1.1", "00:00:00:01:01:01", 1, 1} +}; + +IpamInfo ipam_info[] = { + {"8.1.1.0", 24, "8.1.1.10", true}, +}; + +class ArpPathPreferenceTest : public ::testing::Test { +public: + ArpPathPreferenceTest(): agent(Agent::GetInstance()) {} + virtual void SetUp() { + CreateVmportEnv(input1, 1); + client->WaitForIdle(); + EXPECT_TRUE(VmPortActive(1)); + AddIPAM("vn1", ipam_info, 1); + client->WaitForIdle(); + } + + virtual void TearDown() { + DeleteVmportEnv(input1, 1, true); + DelIPAM("vn1"); + client->WaitForIdle(); + EXPECT_TRUE(VrfGet("vrf1", true) == NULL); + } + + const ArpVrfState* GetVrfState(uint32_t vrf_id) { + const VrfEntry *vrf = agent->vrf_table()->FindVrfFromId(vrf_id); + return static_cast( + vrf->GetState(agent->vrf_table(), + agent->GetArpProto()-> + vrf_table_listener_id())); + } + + bool CheckIpMap(uint32_t vrf_id, uint32_t id, const IpAddress &ip) { + const ArpVrfState *state = GetVrfState(vrf_id); + return state->Get(ip)->IntfPresentInIpMap(id); + } + + bool CheckEvpnMap(uint32_t vrf_id, uint32_t id, const IpAddress &ip) { + const ArpVrfState *state = GetVrfState(vrf_id); + return state->Get(ip)->IntfPresentInEvpnMap(id); + } + + uint32_t GetIpMapRetryCount(uint32_t vrf_id, uint32_t id, const IpAddress &ip) { + const ArpVrfState *state = GetVrfState(vrf_id); + return state->Get(ip)->IntfRetryCountInIpMap(id); + } + + uint32_t GetEvpnMapRetryCount(uint32_t vrf_id, uint32_t id, const IpAddress &ip) { + const ArpVrfState *state = GetVrfState(vrf_id); + return state->Get(ip)->IntfRetryCountInEvpnMap(id); + } +protected: + Agent *agent; +}; + +//Check EVPN route and IP route list has +//interface ID +TEST_F(ArpPathPreferenceTest, Test1) { + uint32_t vrf_id = VrfGet("vrf1")->vrf_id(); + uint32_t intf_id = VmPortGetId(1); + const IpAddress ip = Ip4Address::from_string("8.1.1.1"); + EXPECT_TRUE(CheckIpMap(vrf_id, intf_id, ip)); + EXPECT_TRUE(CheckEvpnMap(vrf_id, intf_id, ip)); + + EXPECT_TRUE(GetIpMapRetryCount(vrf_id, intf_id, ip) >= 1); + EXPECT_TRUE(GetEvpnMapRetryCount(vrf_id, intf_id, ip) >= 1); +} + +//Enqueue Traffic seen for IP route, verify +//EVPN route still has interface ID in map +TEST_F(ArpPathPreferenceTest, Test2) { + CreateVmportEnv(input2, 1); + client->WaitForIdle(); + + uint32_t vrf_id = VrfGet("vrf1")->vrf_id(); + uint32_t intf_id = VmPortGetId(1); + uint32_t intf_id1 = VmPortGetId(2); + const IpAddress ip = Ip4Address::from_string("8.1.1.1"); + + EXPECT_TRUE(CheckIpMap(vrf_id, intf_id, ip)); + EXPECT_TRUE(CheckEvpnMap(vrf_id, intf_id, ip)); + EXPECT_TRUE(GetIpMapRetryCount(vrf_id, intf_id, ip) >= 2); + EXPECT_TRUE(GetEvpnMapRetryCount(vrf_id, intf_id, ip) >= 2); + + EXPECT_TRUE(CheckIpMap(vrf_id, intf_id1, ip)); + EXPECT_TRUE(CheckEvpnMap(vrf_id, intf_id1, ip)); + EXPECT_TRUE(GetIpMapRetryCount(vrf_id, intf_id1, ip) >= 1); + EXPECT_TRUE(GetEvpnMapRetryCount(vrf_id, intf_id1, ip) >= 1); + + DeleteVmportEnv(input2, 1, false); + client->WaitForIdle(); +} + +TEST_F(ArpPathPreferenceTest, Test3) { + uint32_t vrf_id = VrfGet("vrf1")->vrf_id(); + uint32_t intf_id = VmPortGetId(1); + const Ip4Address ip = Ip4Address::from_string("8.1.1.1"); + EXPECT_TRUE(CheckIpMap(vrf_id, intf_id, ip)); + EXPECT_TRUE(CheckEvpnMap(vrf_id, intf_id, ip)); + + EXPECT_TRUE(GetIpMapRetryCount(vrf_id, intf_id, ip) >= 1); + EXPECT_TRUE(GetEvpnMapRetryCount(vrf_id, intf_id, ip) >= 1); + + agent->oper_db()->route_preference_module()-> + EnqueueTrafficSeen(ip, 32, intf_id, vrf_id, + MacAddress::ZeroMac()); + client->WaitForIdle(); + + EXPECT_FALSE(CheckIpMap(vrf_id, intf_id, ip)); + EXPECT_TRUE(CheckEvpnMap(vrf_id, intf_id, ip)); + EXPECT_TRUE(GetEvpnMapRetryCount(vrf_id, intf_id, ip) >= 2); +} + + +int main(int argc, char *argv[]) { + GETUSERARGS(); + + client = TestInit(init_file, ksync_init, true, true); + usleep(100000); + client->WaitForIdle(); + + int ret = RUN_ALL_TESTS(); + TestShutdown(); + delete client; + return ret; +} diff --git a/src/vnsw/agent/vrouter/ksync/route_ksync.cc b/src/vnsw/agent/vrouter/ksync/route_ksync.cc index 2e7a3fe0fc1..cbd5b49553b 100644 --- a/src/vnsw/agent/vrouter/ksync/route_ksync.cc +++ b/src/vnsw/agent/vrouter/ksync/route_ksync.cc @@ -428,8 +428,16 @@ bool RouteKSyncEntry::Sync(DBEntry *e) { const InetUnicastRouteEntry *uc_rt = static_cast(e); MacAddress mac = MacAddress::ZeroMac(); + bool wait_for_traffic = false; if (obj->RouteNeedsMacBinding(uc_rt)) { mac = obj->GetIpMacBinding(uc_rt->vrf(), addr_); + wait_for_traffic = obj->GetIpMacBindingWaitForTraffic(uc_rt->vrf(), + addr_); + } + + if (wait_for_traffic_ == false && wait_for_traffic_ != wait_for_traffic) { + wait_for_traffic_ = wait_for_traffic; + ret = true; } if (mac != mac_) { @@ -814,7 +822,7 @@ void VrfKSyncObject::EvpnRouteTableNotify(DBTablePartBase *partition, evpn_rt->mac()); } else { AddIpMacBinding(evpn_rt->vrf(), evpn_rt->ip_addr(), - evpn_rt->mac()); + evpn_rt->mac(), evpn_rt->WaitForTraffic()); } return; } @@ -918,13 +926,23 @@ void VrfKSyncObject::NotifyUcRoute(VrfEntry *vrf, VrfState *state, } void VrfKSyncObject::AddIpMacBinding(VrfEntry *vrf, const IpAddress &ip, - const MacAddress &mac) { + const MacAddress &mac, + bool wait_for_traffic) { VrfState *state = static_cast (vrf->GetState(vrf->get_table(), vrf_listener_id_)); - if (state == NULL) + if (state == NULL) { return; + } + + IpToMacBinding::iterator it = state->ip_mac_binding_.find(ip); + if (it == state->ip_mac_binding_.end()) { + state->ip_mac_binding_[ip] = + MacBindingInfo(mac, wait_for_traffic); + } else { + it->second.set_mac(mac); + it->second.set_wait_for_traffic(wait_for_traffic); + } - state->ip_mac_binding_[ip] = mac; NotifyUcRoute(vrf, state, ip); } @@ -950,5 +968,21 @@ MacAddress VrfKSyncObject::GetIpMacBinding(VrfEntry *vrf, if (it == state->ip_mac_binding_.end()) return MacAddress::ZeroMac(); - return it->second; + return it->second.mac(); +} + +bool VrfKSyncObject::GetIpMacBindingWaitForTraffic(VrfEntry *vrf, + const IpAddress &ip) const { + VrfState *state = static_cast + (vrf->GetState(vrf->get_table(), vrf_listener_id_)); + if (state == NULL) { + return false; + } + + IpToMacBinding::const_iterator it = state->ip_mac_binding_.find(ip); + if (it == state->ip_mac_binding_.end()) { + return false; + } + + return it->second.wait_for_traffic(); } diff --git a/src/vnsw/agent/vrouter/ksync/route_ksync.h b/src/vnsw/agent/vrouter/ksync/route_ksync.h index 8b51fb5a690..efeca0c1f2c 100644 --- a/src/vnsw/agent/vrouter/ksync/route_ksync.h +++ b/src/vnsw/agent/vrouter/ksync/route_ksync.h @@ -118,10 +118,38 @@ class RouteKSyncObject : public KSyncDBObject { DISALLOW_COPY_AND_ASSIGN(RouteKSyncObject); }; +class MacBindingInfo { +public: + MacBindingInfo(const MacAddress &mac, bool wait_for_traffic): + mac_(mac), wait_for_traffic_(wait_for_traffic) { + } + MacBindingInfo() {} + + bool wait_for_traffic() const { + return wait_for_traffic_; + } + + const MacAddress& mac() const { + return mac_; + } + + void set_mac(const MacAddress &mac) { + mac_ = mac; + } + + void set_wait_for_traffic(bool val) { + wait_for_traffic_ = val; + } + +private: + MacAddress mac_; + bool wait_for_traffic_; +}; + class VrfKSyncObject { public: // Table to maintain IP - MAC binding. Used to stitch MAC to inet routes - typedef std::map IpToMacBinding; + typedef std::map IpToMacBinding; struct VrfState : DBState { VrfState() : DBState(), seen_(false), @@ -148,10 +176,12 @@ class VrfKSyncObject { void UnRegisterEvpnRouteTableListener(const VrfEntry *entry, VrfState *state); void AddIpMacBinding(VrfEntry *vrf, const IpAddress &ip, - const MacAddress &mac); + const MacAddress &mac, bool wait_for_traffic); void DelIpMacBinding(VrfEntry *vrf, const IpAddress &ip, const MacAddress &mac); MacAddress GetIpMacBinding(VrfEntry *vrf, const IpAddress &ip) const; + bool GetIpMacBindingWaitForTraffic(VrfEntry *vrf, + const IpAddress &ip) const; void NotifyUcRoute(VrfEntry *vrf, VrfState *state, const IpAddress &ip); bool RouteNeedsMacBinding(const InetUnicastRouteEntry *rt); DBTableBase::ListenerId vrf_listener_id() const {return vrf_listener_id_;} diff --git a/src/vnsw/agent/vrouter/ksync/test/test_ksync_route.cc b/src/vnsw/agent/vrouter/ksync/test/test_ksync_route.cc index af326d1b2c6..705177e8a77 100644 --- a/src/vnsw/agent/vrouter/ksync/test/test_ksync_route.cc +++ b/src/vnsw/agent/vrouter/ksync/test/test_ksync_route.cc @@ -8,6 +8,7 @@ #include "testing/gunit.h" #include "test/test_cmn_util.h" +#include "oper/path_preference.h" #include "vrouter/ksync/route_ksync.h" struct PortInfo input[] = { @@ -422,6 +423,34 @@ TEST_F(TestKSyncRoute, ecmp_ipam_subnet_route_2) { client->WaitForIdle(); } +TEST_F(TestKSyncRoute, evpn_wait_for_traffic) { + //Send traffic for inet route only + //such that EVPN route is wait_for_traffic state + //and verify ...ip route would be still in wait for traffic state + InetUnicastRouteEntry *rt = vrf1_uc_table_->FindLPM(vnet1_->primary_ip_addr()); + EXPECT_TRUE(rt != NULL); + EXPECT_TRUE(vrf1_obj_->GetIpMacBindingWaitForTraffic(vrf1_, + vnet1_->primary_ip_addr())); + + Agent::GetInstance()->oper_db()->route_preference_module()-> + EnqueueTrafficSeen(vnet1_->primary_ip_addr(), 32, + vnet1_->id(), vnet1_->vrf()->vrf_id(), + MacAddress::ZeroMac()); + client->WaitForIdle(); + + EXPECT_TRUE(vrf1_obj_->GetIpMacBindingWaitForTraffic(vrf1_, + vnet1_->primary_ip_addr())); + + Agent::GetInstance()->oper_db()->route_preference_module()-> + EnqueueTrafficSeen(vnet1_->primary_ip_addr(), 32, + vnet1_->id(), vnet1_->vrf()->vrf_id(), + MacAddress::FromString(vnet1_->vm_mac())); + client->WaitForIdle(); + + EXPECT_FALSE(vrf1_obj_->GetIpMacBindingWaitForTraffic(vrf1_, + vnet1_->primary_ip_addr())); +} + int main(int argc, char **argv) { GETUSERARGS();