Skip to content

Commit

Permalink
Handle VLAN property not set in logical-interface
Browse files Browse the repository at this point in the history 
We do not allow modificaiton of vlan-tag in logical-interface. But, in
IFMap messages its possible that VLAN-Tag property is not set. In
such case, we were setting vlan-tag to 0 and then change the value later
when property is received.

Modified agent code to ignore IFMap messages when vlan-tag property is
not set.

Change-Id: I7447a757c7a15367ffb75e881e266379b26c1abc
Fixes-bug: #1437105
  • Loading branch information
@praveenkv
praveenkv committed Apr 5, 2015
1 parent d978ee5 commit 5963bca
Show file tree
Hide file tree
Showing 8 changed files with 72 additions and 39 deletions.
6 changes: 6 additions & 0 deletions src/vnsw/agent/cmn/agent_cmn.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,13 +81,19 @@ static inline void CloseTaskFds(void) {
}

extern SandeshTraceBufferPtr OperDBTraceBuf;
extern SandeshTraceBufferPtr OperConfigTraceBuf;
extern bool GetBuildInfo(std::string &build_info_str);

#define OPER_TRACE(obj, ...)\
do {\
Oper##obj::TraceMsg(OperDBTraceBuf, __FILE__, __LINE__, __VA_ARGS__);\
} while (false);\

#define OPER_IFMAP_TRACE(obj, ...)\
do {\
Oper##obj::TraceMsg(OperConfigTraceBuf, __FILE__, __LINE__, __VA_ARGS__);\
} while (false);\

#define IFMAP_ERROR(obj, ...)\
do {\
if (LoggingDisabled()) break;\
Expand Down
10 changes: 10 additions & 0 deletions src/vnsw/agent/oper/agent.sandesh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -963,6 +963,16 @@ traceobject sandesh OperInterface {
1: InterfaceInfo intf_info;
}

struct OperConfigInfo {
1: string name;
2: string uuid;
3: string message;
}

traceobject sandesh OperConfig {
1: OperConfigInfo info;
}

traceobject sandesh OperRoute {
1: RouteInfo route_info;
}
Expand Down
54 changes: 28 additions & 26 deletions src/vnsw/agent/oper/logical_interface.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,10 +87,6 @@ bool LogicalInterface::OnChange(const InterfaceTable *table,
ret = true;
}

if (Copy(table, data) == true) {
ret = true;
}

return ret;
}

Expand Down Expand Up @@ -145,8 +141,9 @@ LogicalInterfaceData::~LogicalInterfaceData() {
// VlanLogicalInterface routines
//////////////////////////////////////////////////////////////////////////////
VlanLogicalInterface::VlanLogicalInterface(const boost::uuids::uuid &uuid,
const std::string &name) :
LogicalInterface(uuid, name) {
const std::string &name,
uint16_t vlan) :
LogicalInterface(uuid, name), vlan_(vlan) {
}

VlanLogicalInterface::~VlanLogicalInterface() {
Expand All @@ -157,20 +154,6 @@ DBEntryBase::KeyPtr VlanLogicalInterface::GetDBRequestKey() const {
return DBEntryBase::KeyPtr(key);
}

bool VlanLogicalInterface::Copy(const InterfaceTable *table,
const LogicalInterfaceData *d) {
bool ret = false;
const VlanLogicalInterfaceData *data =
static_cast<const VlanLogicalInterfaceData *>(d);

if (vlan_ != data->vlan_) {
vlan_ = data->vlan_;
ret = true;
}

return ret;
}

//////////////////////////////////////////////////////////////////////////////
// VlanLogicalInterfaceKey routines
//////////////////////////////////////////////////////////////////////////////
Expand All @@ -185,15 +168,17 @@ VlanLogicalInterfaceKey::~VlanLogicalInterfaceKey() {
LogicalInterface *
VlanLogicalInterfaceKey::AllocEntry(const InterfaceTable *table)
const {
return new VlanLogicalInterface(uuid_, name_);
return new VlanLogicalInterface(uuid_, name_, 0);
}

LogicalInterface *
VlanLogicalInterfaceKey::AllocEntry(const InterfaceTable *table,
const InterfaceData *d) const {
VlanLogicalInterface *intf = new VlanLogicalInterface(uuid_, name_);
const VlanLogicalInterfaceData *data =
static_cast<const VlanLogicalInterfaceData *>(d);
VlanLogicalInterface *intf = new VlanLogicalInterface(uuid_, name_,
data->vlan_);

intf->OnChange(table, data);
return intf;
}
Expand Down Expand Up @@ -227,7 +212,8 @@ static LogicalInterfaceKey *BuildKey(IFMapNode *node,
}

static LogicalInterfaceData *BuildData(Agent *agent, IFMapNode *node,
const autogen::LogicalInterface *port) {
const uuid &u,
const autogen::LogicalInterface *port) {
// Find link with physical-interface adjacency
string physical_interface;
IFMapNode *adj_node = NULL;
Expand Down Expand Up @@ -260,15 +246,17 @@ static LogicalInterfaceData *BuildData(Agent *agent, IFMapNode *node,
vmi_uuid);
}

string dev_name;
adj_node = agent->cfg_listener()->FindAdjacentIFMapNode
(agent, node, "physical-router");
if (adj_node) {
dev_name = adj_node->name();
if (dev_uuid != nil_uuid()) {
IFMAP_ERROR(LogicalInterfaceConfiguration,
"Both physical-router and physical-interface links for "
"interface:", node->name(),
"physical interface", physical_interface,
"prouter name", adj_node->name());
"prouter name", dev_name);
}
autogen::PhysicalRouter *router =
static_cast<autogen::PhysicalRouter *>(adj_node->GetObject());
Expand All @@ -277,6 +265,18 @@ static LogicalInterfaceData *BuildData(Agent *agent, IFMapNode *node,
dev_uuid);
}

// Logical-Interface must have VLAN-Tag field. Ignore the interface if
// VLAN-Tag is not yet present
if (port->IsPropertySet(autogen::LogicalInterface::VLAN_TAG) == false) {
OperConfigInfo t;
t.set_name(node->name());
t.set_uuid(UuidToString(u));
t.set_message("VLAN-Tag property not set. Ignoring node");

OPER_IFMAP_TRACE(Config, t);
return NULL;
}

return new VlanLogicalInterfaceData(agent, node, port->display_name(),
physical_interface, vmi_uuid, dev_uuid,
port->vlan_tag());
Expand Down Expand Up @@ -320,9 +320,11 @@ bool InterfaceTable::LogicalInterfaceProcessConfig(IFMapNode *node,
}

req.oper = DBRequest::DB_ENTRY_ADD_CHANGE;
req.data.reset(BuildData(agent(), node, port));
req.data.reset(BuildData(agent(), node, u, port));

Enqueue(&req);
if (req.data.get() != NULL) {
Enqueue(&req);
}
VmInterface::LogicalPortSync(this, node);
return false;
}
Expand Down
7 changes: 2 additions & 5 deletions src/vnsw/agent/oper/logical_interface.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,6 @@ class LogicalInterface : public Interface {
virtual bool OnChange(const InterfaceTable *table,
const LogicalInterfaceData *data);

virtual bool Copy(const InterfaceTable *table,
const LogicalInterfaceData *data) = 0;

const std::string &display_name() const { return display_name_; }
Interface *physical_interface() const;
VmInterface *vm_interface() const;
Expand Down Expand Up @@ -97,14 +94,14 @@ struct VlanLogicalInterfaceData : public LogicalInterfaceData {
class VlanLogicalInterface : public LogicalInterface {
public:
explicit VlanLogicalInterface(const boost::uuids::uuid &uuid,
const std::string &name);
const std::string &name, uint16_t vlan);
virtual ~VlanLogicalInterface();
virtual DBEntryBase::KeyPtr GetDBRequestKey() const;

bool Copy(const InterfaceTable *table, const LogicalInterfaceData *d);
uint16_t vlan() const { return vlan_; }

private:
// IMP: vlan_ cannot be changed
uint16_t vlan_;
DISALLOW_COPY_AND_ASSIGN(VlanLogicalInterface);
};
Expand Down
2 changes: 2 additions & 0 deletions src/vnsw/agent/oper/operdb_init.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ using boost::assign::map_list_of;
using boost::assign::list_of;

SandeshTraceBufferPtr OperDBTraceBuf(SandeshTraceBufferCreate("Oper DB", 5000));
SandeshTraceBufferPtr OperConfigTraceBuf(SandeshTraceBufferCreate("OperIfmap",
1000));

template<typename T>
T *DBTableCreate(DB *db, Agent *agent, OperDB *oper,
Expand Down
15 changes: 11 additions & 4 deletions src/vnsw/agent/oper/test/logical-interface.xml
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<?xml version="1.0"?>
<test_suite name="logical-interface">
<test name="test1">
<test name="test1" verbose="0">
<physical-router uuid="1" name="router-1"/>
<physical-interface uuid="1" name="p-intf-1" device="router-1" />

Expand All @@ -17,7 +17,14 @@
<validate name="validate-1">
<vm-interface name="tap1" uuid="1" present="1" active="1" />
<logical-interface name="l-intf-1" uuid="1" port="p-intf-1" vmi="1"
present="1" />
present="no" />
</validate>

<logical-interface uuid="1" name="l-intf-1" port="p-intf-1" vmi="tap1"
vlan="1"/>
<validate name="validate-1">
<logical-interface name="l-intf-1" uuid="1" port="p-intf-1" vmi="1"
vlan="1" present="1" />
</validate>

<vmi-vrf uuid="1" name="tap1-vm1" del="1"/>
Expand Down Expand Up @@ -52,9 +59,9 @@
<test name="test2">
<physical-router uuid="1" name="router-1"/>
<physical-interface uuid="1" name="p-intf-1" device="router-1" />
<logical-interface uuid="1" name="l-intf-1" port="p-intf-1" />
<logical-interface uuid="1" name="l-intf-1" port="p-intf-1" vlan="10"/>
<validate name="validate-3">
<logical-interface name="l-intf-1" uuid="1" present="1" />
<logical-interface name="l-intf-1" uuid="1" vlan="10" present="1" />
</validate>

<link left="physical-interface" left-name="p-intf-1"
Expand Down
16 changes: 12 additions & 4 deletions src/vnsw/agent/oper/test/test_xml_physical_device.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ string AgentUtXmlRemotePhysicalInterface::NodeType() {
AgentUtXmlLogicalInterface::AgentUtXmlLogicalInterface
(const string &name, const uuid &id, const xml_node &node,
AgentUtXmlTestCase *test_case) :
AgentUtXmlConfig(name, id, node, test_case) {
AgentUtXmlConfig(name, id, node, test_case), vlan_(0xFFFF) {
}

AgentUtXmlLogicalInterface::~AgentUtXmlLogicalInterface() {
Expand All @@ -225,13 +225,16 @@ bool AgentUtXmlLogicalInterface::ReadXml() {

GetStringAttribute(node(), "port", &port_name_);
GetStringAttribute(node(), "vmi", &vmi_name_);
GetUintAttribute(node(), "vlan", &vlan_);
return true;
}

bool AgentUtXmlLogicalInterface::ToXml(xml_node *parent) {
xml_node n = AddXmlNodeWithAttr(parent, NodeType().c_str());
AddXmlNodeWithValue(&n, "name", name());
AddXmlNodeWithValue(&n, "display-name", name());
if (vlan_ >= 0 && vlan_ <= 0x4096)
AddXmlNodeWithIntValue(&n, "logical-interface-vlan-tag", vlan_);
AddIdPerms(&n);

if (port_name_ != "") {
Expand Down Expand Up @@ -390,7 +393,7 @@ const string AgentUtXmlRemotePhysicalInterfaceValidate::ToString() {
AgentUtXmlLogicalInterfaceValidate::AgentUtXmlLogicalInterfaceValidate
(const string &name, const uuid &id, const xml_node &node) :
AgentUtXmlValidationNode(name, node), id_(id), physical_port_(),
device_uuid_(), vmi_uuid_(), vlan_() {
device_uuid_(), vmi_uuid_(), vlan_(0xFFFF) {
}

AgentUtXmlLogicalInterfaceValidate::~AgentUtXmlLogicalInterfaceValidate() {
Expand All @@ -415,10 +418,10 @@ bool AgentUtXmlLogicalInterfaceValidate::ReadXml() {
bool AgentUtXmlLogicalInterfaceValidate::Validate() {
Agent *agent = Agent::GetInstance();

LogicalInterface *port;
VlanLogicalInterface *port;

VlanLogicalInterfaceKey key(id_, name());
port = static_cast<LogicalInterface *>
port = static_cast<VlanLogicalInterface *>
(agent->interface_table()->FindActiveEntry(&key));

if (present() == false) {
Expand Down Expand Up @@ -447,6 +450,11 @@ bool AgentUtXmlLogicalInterfaceValidate::Validate() {
return false;
}

if (vlan_ >= 0 && vlan_ < 4096) {
if (vlan_ != port->vlan())
return false;
}

return true;
}

Expand Down
1 change: 1 addition & 0 deletions src/vnsw/agent/oper/test/test_xml_physical_device.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ class AgentUtXmlLogicalInterface : public AgentUtXmlConfig {
private:
std::string port_name_;
std::string vmi_name_;
uint16_t vlan_;
};

/////////////////////////////////////////////////////////////////////////////
Expand Down

0 comments on commit 5963bca

Please sign in to comment.