Skip to content

Commit

Permalink
Problem:
Browse files Browse the repository at this point in the history
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
  • Loading branch information
jayaramsatya committed Jan 16, 2017
1 parent 5b00aab commit c537a30
Show file tree
Hide file tree
Showing 7 changed files with 125 additions and 58 deletions.
32 changes: 21 additions & 11 deletions src/vnsw/agent/services/arp_entry.cc
Expand Up @@ -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<const VmInterface *>(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_++;
Expand Down Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion src/vnsw/agent/services/arp_entry.h
Expand Up @@ -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();
Expand Down
36 changes: 23 additions & 13 deletions src/vnsw/agent/services/arp_handler.cc
Expand Up @@ -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: {
Expand All @@ -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;
}

Expand Down Expand Up @@ -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,
Expand All @@ -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;

Expand Down
4 changes: 2 additions & 2 deletions src/vnsw/agent/services/arp_handler.h
Expand Up @@ -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);

Expand Down
86 changes: 64 additions & 22 deletions src/vnsw/agent/services/arp_proto.cc
Expand Up @@ -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);
Expand All @@ -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_);
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -367,14 +373,11 @@ void ArpVrfState::RouteUpdate(DBTablePartBase *part, DBEntryBase *entry) {
ArpDBState *state =
static_cast<ArpDBState *>
(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;
Expand All @@ -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<const InterfaceNH *>(
route->GetActiveNextHop());
if (intf_nh) {
const Interface *intf =
static_cast<const Interface *>(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
Expand Down Expand Up @@ -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<const ArpVrfState *>
(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,
Expand Down
11 changes: 6 additions & 5 deletions src/vnsw/agent/services/arp_proto.h
Expand Up @@ -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++; }
Expand Down Expand Up @@ -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_;
Expand Down
12 changes: 8 additions & 4 deletions src/vnsw/agent/services/test/arp_test.cc
Expand Up @@ -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
Expand Down

0 comments on commit c537a30

Please sign in to comment.