From 65bfda4275e660c80805121fb0ebbdc07a88b52f 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 --- 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 6f60877bdce..a5733e961b0 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_); @@ -165,8 +171,8 @@ bool ArpPathPreferenceState::SendArpRequest(WaitForTrafficIntfMap } 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; } @@ -367,14 +373,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; @@ -390,11 +393,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 @@ -602,19 +618,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 6e76d656c92..09a0de8b3a2 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++; } @@ -173,11 +174,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