From 3192a79712e8e2c85e2faf50005b3631bb6685e6 Mon Sep 17 00:00:00 2001 From: jayaramsatya Date: Wed, 11 Jan 2017 19:33:37 +0530 Subject: [PATCH] Problem: For vcenter only mode, the agent replies arp with the physical intf mac address for the VN's gateway ip. When vm migrates to a different esxi host as part of vmotion,vcenter plugin binds it to the new contrail-compute-vm,whose physical intf mac is different. But the vm still hold the old mac address for the gateway ip in the arp table. Until the old mac address gets flushed and the new mac is learnt for the gateway ip, the traffic is lost from the vm for a different subnet. Solution: Whenever VM migrates to new esxi host. and upon creation of VM interface Arp module will listen on route update. after getting the route update send the gratious arp for the Gateway IP. closes-bug:#1650219 Change-Id: I6bf4bab98c956bead866d22b7c8e8960419315b4 (cherry picked from commit 5912a8bf417658f53d7878cc800b14ade7800f64) --- src/vnsw/agent/services/arp_entry.cc | 32 ++++++--- src/vnsw/agent/services/arp_entry.h | 2 +- src/vnsw/agent/services/arp_handler.cc | 36 ++++++---- src/vnsw/agent/services/arp_handler.h | 4 +- src/vnsw/agent/services/arp_proto.cc | 86 ++++++++++++++++++------ src/vnsw/agent/services/arp_proto.h | 11 +-- src/vnsw/agent/services/test/arp_test.cc | 12 ++-- 7 files changed, 125 insertions(+), 58 deletions(-) diff --git a/src/vnsw/agent/services/arp_entry.cc b/src/vnsw/agent/services/arp_entry.cc index 13898c1e449..7a179256d7d 100644 --- a/src/vnsw/agent/services/arp_entry.cc +++ b/src/vnsw/agent/services/arp_entry.cc @@ -130,17 +130,27 @@ void ArpEntry::SendGratuitousArp() { Agent *agent = handler_->agent(); ArpProto *arp_proto = agent->GetArpProto(); if (agent->router_id_configured()) { - handler_->SendArp(ARPOP_REQUEST, arp_proto->ip_fabric_interface_mac(), - agent->router_id().to_ulong(), MacAddress(), - agent->router_id().to_ulong(), - arp_proto->ip_fabric_interface_index(), - key_.vrf->vrf_id()); - } + if (interface_->type() == Interface::VM_INTERFACE) { + const VmInterface *vmi = static_cast(interface_); + MacAddress smac = vmi->GetVifMac(agent); + if (key_.vrf && key_.vrf->vn()) { + IpAddress gw_ip = key_.vrf->vn()->GetGatewayFromIpam + (Ip4Address(key_.ip)); + if (!gw_ip.is_unspecified() && gw_ip.is_v4()) { + handler_->SendArp(ARPOP_REQUEST, smac, + gw_ip.to_v4().to_ulong(), + smac, vmi->mac(), gw_ip.to_v4().to_ulong(), + vmi->id(), key_.vrf->vrf_id()); + } + } + } else { + handler_->SendArp(ARPOP_REQUEST, arp_proto->ip_fabric_interface_mac(), + agent->router_id().to_ulong(), MacAddress(), + MacAddress::BroadcastMac(), agent->router_id().to_ulong(), + arp_proto->ip_fabric_interface_index(), + key_.vrf->vrf_id()); + } - if (retry_count_ == ArpProto::kGratRetries) { - // Retaining this entry till arp module is deleted - // arp_proto->DelGratuitousArpEntry(); - return; } retry_count_++; @@ -193,7 +203,7 @@ void ArpEntry::SendArpRequest() { if (vrf_id != VrfEntry::kInvalidIndex) { handler_->SendArp(ARPOP_REQUEST, smac, ip.to_ulong(), - MacAddress(), key_.ip, intf_id, vrf_id); + MacAddress(), MacAddress::BroadcastMac(), key_.ip, intf_id, vrf_id); } StartTimer(arp_proto->retry_timeout(), ArpProto::RETRY_TIMER_EXPIRED); diff --git a/src/vnsw/agent/services/arp_entry.h b/src/vnsw/agent/services/arp_entry.h index 83f3c205d62..12654510f70 100644 --- a/src/vnsw/agent/services/arp_entry.h +++ b/src/vnsw/agent/services/arp_entry.h @@ -47,7 +47,7 @@ class ArpEntry { bool IsResolved(); void Resync(bool policy, const VnListType &vnlist, const SecurityGroupList &sg); - + int retry_count() const { return retry_count_; } private: void StartTimer(uint32_t timeout, uint32_t mtype); void SendArpRequest(); diff --git a/src/vnsw/agent/services/arp_handler.cc b/src/vnsw/agent/services/arp_handler.cc index 677d4828c5b..66405f86541 100644 --- a/src/vnsw/agent/services/arp_handler.cc +++ b/src/vnsw/agent/services/arp_handler.cc @@ -248,14 +248,18 @@ bool ArpHandler::HandleMessage() { } case ArpProto::ARP_SEND_GRATUITOUS: { - if (!arp_proto->gratuitous_arp_entry()) { - arp_proto->set_gratuitous_arp_entry( - new ArpEntry(io_, this, ipc->key, ipc->key.vrf, - ArpEntry::ACTIVE, ipc->interface)); - ret = false; + bool key_valid = false; + ArpProto::ArpIterator it = + arp_proto->GratiousArpEntryIterator(ipc->key, &key_valid); + if (key_valid) { + if (it->second == NULL) { + it->second = new ArpEntry(io_, this, ipc->key, ipc->key.vrf, + ArpEntry::ACTIVE, ipc->interface); + ret = false; + } + it->second->SendGratuitousArp(); + break; } - arp_proto->gratuitous_arp_entry()->SendGratuitousArp(); - break; } case ArpProto::ARP_DELETE: { @@ -280,8 +284,15 @@ bool ArpHandler::HandleMessage() { } case ArpProto::GRATUITOUS_TIMER_EXPIRED: { - if (arp_proto->gratuitous_arp_entry()) - arp_proto->gratuitous_arp_entry()->SendGratuitousArp(); + ArpEntry *entry = arp_proto->GratiousArpEntry(ipc->key); + if (entry && entry->retry_count() <= ArpProto::kGratRetries) { + entry->SendGratuitousArp(); + } else { + // Need to validate deleting the Arp entry upon fabric vrf Delete only + if (ipc->key.vrf->GetName() != agent()->fabric_vrf_name()) { + arp_proto->DeleteGratuitousArpEntry(entry); + } + } break; } @@ -320,8 +331,8 @@ uint16_t ArpHandler::ArpHdr(const MacAddress &smac, in_addr_t sip, } void ArpHandler::SendArp(uint16_t op, const MacAddress &smac, in_addr_t sip, - const MacAddress &tmac, in_addr_t tip, - uint32_t itf, uint32_t vrf) { + const MacAddress &tmac, const MacAddress &dmac, + in_addr_t tip, uint32_t itf, uint32_t vrf) { if (pkt_info_->packet_buffer() == NULL) { pkt_info_->AllocPacketBuffer(agent(), PktHandler::ARP, ARP_TX_BUFF_LEN, @@ -332,8 +343,7 @@ void ArpHandler::SendArp(uint16_t op, const MacAddress &smac, in_addr_t sip, memset(buf, 0, pkt_info_->packet_buffer()->data_len()); pkt_info_->eth = (struct ether_header *)buf; int l2_len = EthHdr(buf, pkt_info_->packet_buffer()->data_len(), - itf, smac, MacAddress::BroadcastMac(), ETHERTYPE_ARP); - + itf, smac, dmac, ETHERTYPE_ARP); arp_ = pkt_info_->arp = (ether_arp *) (buf + l2_len); arp_tpa_ = tip; diff --git a/src/vnsw/agent/services/arp_handler.h b/src/vnsw/agent/services/arp_handler.h index acacf14a78f..67b2c354384 100644 --- a/src/vnsw/agent/services/arp_handler.h +++ b/src/vnsw/agent/services/arp_handler.h @@ -20,8 +20,8 @@ class ArpHandler : public ProtoHandler { bool Run(); void SendArp(uint16_t op, const MacAddress &smac, in_addr_t sip, - const MacAddress &tmac, in_addr_t tip, - uint32_t itf, uint32_t vrf); + const MacAddress &tmac, const MacAddress &dmac, + in_addr_t tip, uint32_t itf, uint32_t vrf); friend void intrusive_ptr_add_ref(const ArpHandler *p); friend void intrusive_ptr_release(const ArpHandler *p); diff --git a/src/vnsw/agent/services/arp_proto.cc b/src/vnsw/agent/services/arp_proto.cc index da452172108..1eb74fa7189 100644 --- a/src/vnsw/agent/services/arp_proto.cc +++ b/src/vnsw/agent/services/arp_proto.cc @@ -19,9 +19,8 @@ ArpProto::ArpProto(Agent *agent, boost::asio::io_service &io, bool run_with_vrouter) : Proto(agent, "Agent::Services", PktHandler::ARP, io), run_with_vrouter_(run_with_vrouter), ip_fabric_interface_index_(-1), - ip_fabric_interface_(NULL), gratuitous_arp_entry_(NULL), - max_retries_(kMaxRetries), retry_timeout_(kRetryTimeout), - aging_timeout_(kAgingTimeout) { + ip_fabric_interface_(NULL), max_retries_(kMaxRetries), + retry_timeout_(kRetryTimeout), aging_timeout_(kAgingTimeout) { // limit the number of entries in the workqueue work_queue_.SetSize(agent->params()->services_queue_limit()); work_queue_.SetBounded(true); @@ -39,11 +38,18 @@ ArpProto::~ArpProto() { } void ArpProto::Shutdown() { - del_gratuitous_arp_entry(); // we may have arp entries in arp cache without ArpNH, empty them for (ArpIterator it = arp_cache_.begin(); it != arp_cache_.end(); ) { it = DeleteArpEntry(it); } + + for (ArpIterator it = gratuitous_arp_cache_.begin(); + it != gratuitous_arp_cache_.end();) { + ArpEntry *entry = it->second; + gratuitous_arp_cache_.erase(it++); + if (entry) + delete entry; + } agent_->vrf_table()->Unregister(vrf_table_listener_id_); agent_->interface_table()->Unregister(interface_table_listener_id_); agent_->nexthop_table()->Unregister(nexthop_table_listener_id_); @@ -128,8 +134,8 @@ bool ArpDBState::SendArpRequest() { it->second++; arp_handler.SendArp(ARPOP_REQUEST, smac, gw_ip_.to_v4().to_ulong(), - MacAddress(), vm_ip_.to_v4().to_ulong(), - it->first, vrf_id_); + MacAddress(), MacAddress::BroadcastMac(), + vm_ip_.to_v4().to_ulong(), it->first, vrf_id_); vrf_state_->arp_proto->IncrementStatsVmArpReq(); ret = true; } @@ -271,14 +277,11 @@ void ArpVrfState::RouteUpdate(DBTablePartBase *part, DBEntryBase *entry) { ArpDBState *state = static_cast (entry->GetState(part->parent(), route_table_listener_id)); - + ArpKey key(route->addr().to_v4().to_ulong(), route->vrf()); + ArpEntry *arpentry = arp_proto->GratiousArpEntry(key); if (entry->IsDeleted() || deleted) { if (state) { - if (arp_proto->gratuitous_arp_entry() && - arp_proto->gratuitous_arp_entry()->key().ip == - route->addr().to_v4().to_ulong()) { - arp_proto->del_gratuitous_arp_entry(); - } + arp_proto->DeleteGratuitousArpEntry(arpentry); entry->ClearState(part->parent(), route_table_listener_id); state->Delete(route); delete state; @@ -294,11 +297,24 @@ void ArpVrfState::RouteUpdate(DBTablePartBase *part, DBEntryBase *entry) { if (route->vrf()->GetName() == agent->fabric_vrf_name() && route->GetActiveNextHop()->GetType() == NextHop::RECEIVE) { - arp_proto->del_gratuitous_arp_entry(); //Send Grat ARP + arp_proto->AddGratuitousArpEntry(key); arp_proto->SendArpIpc(ArpProto::ARP_SEND_GRATUITOUS, route->addr().to_v4().to_ulong(), route->vrf(), arp_proto->ip_fabric_interface()); + } else { + const InterfaceNH *intf_nh = dynamic_cast( + route->GetActiveNextHop()); + if (intf_nh) { + const Interface *intf = + static_cast(intf_nh->GetInterface()); + if (intf->type() == Interface::VM_INTERFACE) { + ArpKey intf_key(route->addr().to_v4().to_ulong(), route->vrf()); + arp_proto->AddGratuitousArpEntry(intf_key); + arp_proto->SendArpIpc(ArpProto::ARP_SEND_GRATUITOUS, + route->addr().to_v4().to_ulong(), intf->vrf(), intf); + } + } } //Check if there is a local VM path, if yes send a @@ -459,19 +475,45 @@ bool ArpProto::TimerExpiry(ArpKey &key, uint32_t timer_type, return false; } -ArpEntry *ArpProto::gratuitous_arp_entry() const { - return gratuitous_arp_entry_; + void ArpProto::AddGratuitousArpEntry(ArpKey &key) { + gratuitous_arp_cache_.insert(ArpCachePair(key, NULL)); } -void ArpProto::set_gratuitous_arp_entry(ArpEntry *entry) { - gratuitous_arp_entry_ = entry; -} +void ArpProto::DeleteGratuitousArpEntry(ArpEntry *entry) { + if (!entry) + return ; -void ArpProto::del_gratuitous_arp_entry() { - if (gratuitous_arp_entry_) { - delete gratuitous_arp_entry_; - gratuitous_arp_entry_ = NULL; + ArpProto::ArpIterator iter = gratuitous_arp_cache_.find(entry->key()); + if (iter == gratuitous_arp_cache_.end()) { + return; } + gratuitous_arp_cache_.erase(iter); + delete entry; +} + +ArpEntry * +ArpProto::GratiousArpEntry(const ArpKey &key) { + ArpProto::ArpIterator it = gratuitous_arp_cache_.find(key); + if (it == gratuitous_arp_cache_.end()) + return NULL; + return it->second; +} +ArpProto::ArpIterator +ArpProto::GratiousArpEntryIterator(const ArpKey &key, bool *key_valid) { + ArpProto::ArpIterator it = gratuitous_arp_cache_.find(key); + if (it == gratuitous_arp_cache_.end()) + return it; + const VrfEntry *vrf = key.vrf; + if (!vrf) + return it; + const ArpVrfState *state = static_cast + (vrf->GetState(vrf->get_table_partition()->parent(), + vrf_table_listener_id_)); + // If VRF is delete marked, do not add ARP entries to cache + if (state == NULL || state->deleted == true) + return it; + *key_valid = true; + return it; } void ArpProto::SendArpIpc(ArpProto::ArpMsgType type, in_addr_t ip, diff --git a/src/vnsw/agent/services/arp_proto.h b/src/vnsw/agent/services/arp_proto.h index a938402d4cf..f111fe70fad 100644 --- a/src/vnsw/agent/services/arp_proto.h +++ b/src/vnsw/agent/services/arp_proto.h @@ -109,10 +109,11 @@ class ArpProto : public Proto { ip_fabric_interface_mac_ = mac; } - ArpEntry *gratuitous_arp_entry() const; - void set_gratuitous_arp_entry(ArpEntry *entry); - void del_gratuitous_arp_entry(); - + void AddGratuitousArpEntry(ArpKey &key); + void DeleteGratuitousArpEntry(ArpEntry *entry); + ArpEntry* GratiousArpEntry (const ArpKey &key); + ArpProto::ArpIterator GratiousArpEntryIterator(const ArpKey & key, + bool *key_valid); void IncrementStatsArpReq() { arp_stats_.arp_req++; } void IncrementStatsArpReplies() { arp_stats_.arp_replies++; } void IncrementStatsGratuitous() { arp_stats_.arp_gratuitous++; } @@ -170,11 +171,11 @@ class ArpProto : public Proto { ArpCache arp_cache_; ArpStats arp_stats_; + ArpCache gratuitous_arp_cache_; bool run_with_vrouter_; uint32_t ip_fabric_interface_index_; MacAddress ip_fabric_interface_mac_; Interface *ip_fabric_interface_; - ArpEntry *gratuitous_arp_entry_; DBTableBase::ListenerId vrf_table_listener_id_; DBTableBase::ListenerId interface_table_listener_id_; DBTableBase::ListenerId nexthop_table_listener_id_; diff --git a/src/vnsw/agent/services/test/arp_test.cc b/src/vnsw/agent/services/test/arp_test.cc index 067d4113bcb..39b1ccbdd2e 100644 --- a/src/vnsw/agent/services/test/arp_test.cc +++ b/src/vnsw/agent/services/test/arp_test.cc @@ -481,26 +481,30 @@ TEST_F(ArpTest, GratArpSendTest) { //Add a vhost rcv route and check that grat arp entry gets created TriggerAddVhostRcvRoute(ip1); client->WaitForIdle(); - EXPECT_TRUE(Agent::GetInstance()->GetArpProto()->gratuitous_arp_entry()->key().ip == ip1.to_ulong()); + const VrfEntry *vrf = Agent::GetInstance()->vrf_table()->FindVrfFromName( + Agent::GetInstance()->fabric_vrf_name()); + ArpKey key(ip1.to_ulong(), vrf); + EXPECT_TRUE(Agent::GetInstance()->GetArpProto()->FindGratiousArpEntry(key) != NULL); Agent::GetInstance()->fabric_inet4_unicast_table()->DeleteReq( Agent::GetInstance()->local_peer(), Agent::GetInstance()->fabric_vrf_name(), ip1, 32, NULL); client->WaitForIdle(); - EXPECT_TRUE(Agent::GetInstance()->GetArpProto()->gratuitous_arp_entry() == NULL); + EXPECT_TRUE(Agent::GetInstance()->GetArpProto()->FindGratiousArpEntry(key) == NULL); Ip4Address ip2 = Ip4Address::from_string("1.1.1.10"); //Add yet another vhost rcv route and check that grat arp entry get created TriggerAddVhostRcvRoute(ip2); client->WaitForIdle(); - EXPECT_TRUE(Agent::GetInstance()->GetArpProto()->gratuitous_arp_entry()->key().ip == ip2.to_ulong()); + ArpKey key1(ip2.to_ulong(), vrf); + EXPECT_TRUE(Agent::GetInstance()->GetArpProto()->FindGratiousArpEntry(key1) != NULL); Agent::GetInstance()->fabric_inet4_unicast_table()->DeleteReq( Agent::GetInstance()->local_peer(), Agent::GetInstance()->fabric_vrf_name(), ip2, 32, NULL); client->WaitForIdle(); - EXPECT_TRUE(Agent::GetInstance()->GetArpProto()->gratuitous_arp_entry() == NULL); + EXPECT_TRUE(Agent::GetInstance()->GetArpProto()->FindGratiousArpEntry(key1) == NULL); } #if 0