Skip to content

Commit

Permalink
Fix issue of interface becoming inactive even when it was active.
Browse files Browse the repository at this point in the history 
Issue
Agent interface was marked as “os_oper_state = down” even when the state of the interface in OS was Up. This can happen when agent index for interface is reused for a different interface and OS notification for old interface comes after notification for new interface.

Fix
As soon as we see the delete of interface, mark its index as invalid in VnswInterfaceListenerBase so that it cannot be used in updates until Add is seen again. Also when add is seen update the index in VnswInterfaceListenerBase before equeuing notification for resync of OS attributes. Also re-read OS index for interface whenever notification enqueued by VnswInterfaceListenerBase is processed.

Also define tracebuffer for logging VnswIf events. Also increase the size of tracebuffer for Config tracebuffer.

Change-Id: Iec418188faf1bb6839deaad41174ccecdcfc5b04
Closes-Bug: #1506997
  • Loading branch information
@ashoksr
ashoksr committed May 21, 2016
1 parent 9c5a4d1 commit fd4e937
Show file tree
Hide file tree
Showing 6 changed files with 90 additions and 24 deletions.
2 changes: 1 addition & 1 deletion src/vnsw/agent/cfg/cfg_init.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ using boost::optional;

void IFMapAgentSandeshInit(DB *, IFMapAgentParser *);

SandeshTraceBufferPtr CfgTraceBuf(SandeshTraceBufferCreate("Config", 100));
SandeshTraceBufferPtr CfgTraceBuf(SandeshTraceBufferCreate("Config", 2000));

AgentConfig::AgentConfig(Agent *agent)
: agent_(agent),
Expand Down
14 changes: 7 additions & 7 deletions src/vnsw/agent/oper/interface.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -433,7 +433,8 @@ void Interface::GetOsParams(Agent *agent) {
assert(fd >= 0);
if (ioctl(fd, SIOCGIFHWADDR, (void *)&ifr) < 0) {
LOG(ERROR, "Error <" << errno << ": " << strerror(errno) <<
"> querying mac-address for interface <" << name << ">");
"> querying mac-address for interface <" << name << "> " <<
"Agent-index <" << id_ << ">");
os_oper_state_ = false;
close(fd);
return;
Expand All @@ -442,7 +443,8 @@ void Interface::GetOsParams(Agent *agent) {

if (ioctl(fd, SIOCGIFFLAGS, (void *)&ifr) < 0) {
LOG(ERROR, "Error <" << errno << ": " << strerror(errno) <<
"> querying mac-address for interface <" << name << ">");
"> querying flags for interface <" << name << "> " <<
"Agent-index <" << id_ << ">");
os_oper_state_ = false;
close(fd);
return;
Expand All @@ -460,11 +462,9 @@ void Interface::GetOsParams(Agent *agent) {
mac_ = ifr.ifr_addr;
#endif

if (os_index_ == kInvalidIndex) {
int idx = if_nametoindex(name.c_str());
if (idx)
os_index_ = idx;
}
int idx = if_nametoindex(name.c_str());
if (idx)
os_index_ = idx;
}

void Interface::SetKey(const DBRequestKey *key) {
Expand Down
9 changes: 9 additions & 0 deletions src/vnsw/agent/vrouter/ksync/agent_ksync.sandesh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,3 +86,12 @@ traceobject sandesh KSyncVxLan {
1: KSyncVxLanInfo info;
}

/**
* @description: Trace message for Vnsw Interface events
* @type: Trace
* @severity: DEBUG
*/
trace sandesh VnswIfInfoTrace {
1: string info;
}

18 changes: 14 additions & 4 deletions src/vnsw/agent/vrouter/ksync/test/test_vnswif.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@ class TestVnswIf : public ::testing::Test {
WAIT_FOR(1000, 100, (VmPortFindRetDel(2) == false));
}

void SetSeen(const string &ifname, bool oper) {
vnswif_->SetSeen(agent_->vhost_interface_name(), true);
void SetSeen(const string &ifname, bool oper, uint32_t id) {
vnswif_->SetSeen(agent_->vhost_interface_name(), true, id);
}

void ResetSeen(const string &ifname) {
Expand Down Expand Up @@ -132,7 +132,12 @@ TEST_F(TestVnswIf, host_route_del) {

// On link flap of vhost, all link-local must be written again
TEST_F(TestVnswIf, vhost_link_flap) {
SetSeen(agent_->vhost_interface_name(), true);
uint32_t id = Interface::kInvalidIndex;
const Interface *intf = agent_->vhost_interface();
if (intf) {
id = intf->id();
}
SetSeen(agent_->vhost_interface_name(), true, id);
client->WaitForIdle();

// Set link-state down
Expand Down Expand Up @@ -177,7 +182,12 @@ TEST_F(TestVnswIf, inactive_1) {

// vhost-ip address update
TEST_F(TestVnswIf, vhost_addr_1) {
SetSeen(agent_->vhost_interface_name(), true);
uint32_t id = Interface::kInvalidIndex;
const Interface *intf = agent_->vhost_interface();
if (intf) {
id = intf->id();
}
SetSeen(agent_->vhost_interface_name(), true, id);
client->WaitForIdle();

VnswInterfaceListener::HostInterfaceEntry *entry =
Expand Down
61 changes: 50 additions & 11 deletions src/vnsw/agent/vrouter/ksync/vnswif_listener_base.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@

extern void RouterIdDepInit(Agent *agent);

SandeshTraceBufferPtr VnswIfTraceBuf(SandeshTraceBufferCreate(
VNSWIF_TRACE_BUF, 2000));

VnswInterfaceListenerBase::VnswInterfaceListenerBase(Agent *agent) :
agent_(agent), read_buf_(NULL), sock_fd_(-1),
sock_(*(agent->event_manager())->io_service()),
Expand Down Expand Up @@ -111,17 +114,28 @@ void VnswInterfaceListenerBase::InterfaceNotify(DBTablePartBase *part,

if (vmport->IsDeleted()) {
if (state) {
HostInterfaceEntry *entry = GetHostInterfaceEntry(vmport->name());
uint32_t id = Interface::kInvalidIndex;
if (entry) {
id = entry->oper_id_;
}
ostringstream oss;
oss << "Intf Del " << vmport->name() << " id " << id;
string msg = oss.str();
VNSWIF_TRACE(msg.c_str());
ResetSeen(vmport->name(), true);
e->ClearState(part->parent(), intf_listener_id_);
delete state;
}
} else {
if (state == NULL) {
ostringstream oss;
oss << "Intf Add " << vmport->name() << " id " << vmport->id();
string msg = oss.str();
VNSWIF_TRACE(msg.c_str());
state = new State(vmport->mdata_ip_addr());
e->SetState(part->parent(), intf_listener_id_, state);
SetSeen(vmport->name(), true);
HostInterfaceEntry *entry = GetHostInterfaceEntry(vmport->name());
entry->oper_id_ = vmport->id();
SetSeen(vmport->name(), true, vmport->id());
} else {
state->addr_ = addr;
}
Expand Down Expand Up @@ -149,10 +163,19 @@ bool VnswInterfaceListenerBase::IsInterfaceActive(const HostInterfaceEntry *entr
}

static void InterfaceResync(Agent *agent, uint32_t id, bool active) {
if (id == Interface::kInvalidIndex) {
return;
}
InterfaceTable *table = agent->interface_table();
Interface *interface = table->FindInterface(id);
if (interface == NULL)
if (interface == NULL) {
ostringstream oss;
oss << "InterfaceResync failed. Interface index " << id <<
" not found. Active " << active;
string msg = oss.str();
VNSWIF_TRACE(msg.c_str());
return;
}

if (agent->test_mode())
interface->set_test_oper_state(active);
Expand Down Expand Up @@ -185,7 +208,8 @@ void VnswInterfaceListenerBase::DeActivate(const std::string &name, uint32_t id)
InterfaceResync(agent_, id, false);
}

void VnswInterfaceListenerBase::SetSeen(const std::string &name, bool oper) {
void VnswInterfaceListenerBase::SetSeen(const std::string &name, bool oper,
uint32_t id) {
HostInterfaceEntry *entry = GetHostInterfaceEntry(name);
if (entry == NULL) {
entry = new HostInterfaceEntry();
Expand All @@ -194,6 +218,7 @@ void VnswInterfaceListenerBase::SetSeen(const std::string &name, bool oper) {

if (oper) {
entry->oper_seen_ = true;
entry->oper_id_ = id;
} else {
entry->host_seen_ = true;
}
Expand All @@ -208,6 +233,7 @@ void VnswInterfaceListenerBase::ResetSeen(const std::string &name, bool oper) {

if (oper) {
entry->oper_seen_ = false;
entry->oper_id_ = Interface::kInvalidIndex;
} else {
entry->host_seen_ = false;
}
Expand All @@ -219,7 +245,9 @@ void VnswInterfaceListenerBase::ResetSeen(const std::string &name, bool oper) {
return;
}

DeActivate(name, entry->oper_id_);
if (!oper) {
DeActivate(name, entry->oper_id_);
}
}

void VnswInterfaceListenerBase::SetLinkState(const std::string &name, bool link_up){
Expand All @@ -240,7 +268,7 @@ void VnswInterfaceListenerBase::HandleInterfaceEvent(const Event *event) {
if (event->event_ == Event::DEL_INTERFACE) {
ResetSeen(event->interface_, false);
} else {
SetSeen(event->interface_, false);
SetSeen(event->interface_, false, Interface::kInvalidIndex);
bool up =
(event->flags_ & (IFF_UP | IFF_RUNNING)) == (IFF_UP | IFF_RUNNING);

Expand Down Expand Up @@ -309,8 +337,11 @@ void VnswInterfaceListenerBase::HandleAddressEvent(const Event *event) {
return;
}

LOG(DEBUG, "Setting IP address for " << event->interface_ << " to "
<< event->addr_.to_string() << "/" << (unsigned short)event->plen_);
ostringstream oss;
oss << "Setting IP address for " << event->interface_ << " to "
<< event->addr_.to_string() << "/" << (unsigned short)event->plen_;
string msg = oss.str();
VNSWIF_TRACE(msg.c_str());
vhost_update_count_++;

bool dep_init_reqd = false;
Expand Down Expand Up @@ -414,11 +445,19 @@ static string EventTypeToString(uint32_t type) {
}

bool VnswInterfaceListenerBase::ProcessEvent(Event *event) {
LOG(DEBUG, "VnswInterfaceListenerBase Event " << EventTypeToString(event->event_)
HostInterfaceEntry *entry = GetHostInterfaceEntry(event->interface_);
uint32_t id = Interface::kInvalidIndex;
if (entry) {
id = entry->oper_id_;
}
ostringstream oss;
oss << " Event " << EventTypeToString(event->event_)
<< " Interface " << event->interface_ << " Addr "
<< event->addr_.to_string() << " prefixlen " << (uint32_t)event->plen_
<< " Gateway " << event->gw_.to_string() << " Flags " << event->flags_
<< " Protocol " << (uint32_t)event->protocol_);
<< " Protocol " << (uint32_t)event->protocol_ << " Index " << id;
string msg = oss.str();
VNSWIF_TRACE(msg.c_str());

switch (event->event_) {
case Event::ADD_ADDR:
Expand Down
10 changes: 9 additions & 1 deletion src/vnsw/agent/vrouter/ksync/vnswif_listener_base.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,14 @@

#define XAPI_INTF_PREFIX "xapi"

#define VNSWIF_TRACE_BUF "VnswIfTrace"

extern SandeshTraceBufferPtr VnswIfTraceBuf;

#define VNSWIF_TRACE(...) do { \
VnswIfInfoTrace::TraceMsg(VnswIfTraceBuf, __FILE__, __LINE__, ##__VA_ARGS__); \
} while (0);

namespace local = boost::asio::local;

class VnswInterfaceListenerBase {
Expand Down Expand Up @@ -140,7 +148,7 @@ class VnswInterfaceListenerBase {
void AddLinkLocalRoutes();
void DelLinkLocalRoutes();

void SetSeen(const std::string &name, bool oper);
void SetSeen(const std::string &name, bool oper, uint32_t oper_idx);
void ResetSeen(const std::string &name, bool oper);
void Activate(const std::string &name, uint32_t os_id);
void DeActivate(const std::string &name, uint32_t os_id);
Expand Down

0 comments on commit fd4e937

Please sign in to comment.