Skip to content

Commit

Permalink
Tor agent crashes during addition of multicast route.
Browse files Browse the repository at this point in the history
Problem:
When VN to VRF link is not present addition of multicast route in tor agent
used to crash as addition routine tries to access VN name from VRF entry.
Absence of link used to return NULL VN and accessing it for VN name
results in crash.

Solution:
Use VN name from VN OVSDB entry. Pass it along in API.
Multicast OVSDB does not gets resolved till VN OVSDB is resolved,
hence using VN name from same is safe.

Conflicts:
	src/vnsw/agent/oper/vn.cc
	src/vnsw/agent/oper/vn.h
	src/vnsw/agent/test/test_util.cc

Change-Id: If9cf04ba8a424957dcdb9559d0b99b67d868d32f
Closes-bug: 1443318
  • Loading branch information
manishsing committed May 13, 2015
1 parent 9d8be4a commit a904a76
Show file tree
Hide file tree
Showing 14 changed files with 378 additions and 17 deletions.
3 changes: 2 additions & 1 deletion src/vnsw/agent/oper/bridge_route.cc
Expand Up @@ -129,6 +129,7 @@ void BridgeAgentRouteTable::AddBridgeRoute(const AgentRoute *rt) {

void BridgeAgentRouteTable::AddOvsPeerMulticastRoute(const Peer *peer,
uint32_t vxlan_id,
const std::string &vn_name,
Ip4Address tsn,
Ip4Address tor_ip) {
const VrfEntry *vrf = vrf_entry();
Expand All @@ -142,7 +143,7 @@ void BridgeAgentRouteTable::AddOvsPeerMulticastRoute(const Peer *peer,
vrf->GetName(),
MacAddress::BroadcastMac(),
vxlan_id));
req.data.reset(new MulticastRoute(vrf->vn()->GetName(), 0, vxlan_id,
req.data.reset(new MulticastRoute(vn_name, 0, vxlan_id,
TunnelType::VxlanType(),
nh_req, Composite::L2COMP));
BridgeTableProcess(agent(), vrf_name(), req);
Expand Down
1 change: 1 addition & 0 deletions src/vnsw/agent/oper/bridge_route.h
Expand Up @@ -30,6 +30,7 @@ class BridgeAgentRouteTable : public AgentRouteTable {
const VmInterface *vm_intf);
void AddOvsPeerMulticastRoute(const Peer* peer,
uint32_t vxlan_id,
const std::string &vn_name,
Ip4Address vtep,
Ip4Address tor_ip);
void AddBridgeRoute(const AgentRoute *rt);
Expand Down
4 changes: 2 additions & 2 deletions src/vnsw/agent/oper/vn.cc
Expand Up @@ -786,14 +786,14 @@ bool VnTable::IFNodeToReq(IFMapNode *node, DBRequest &req) {
void VnTable::AddVn(const uuid &vn_uuid, const string &name,
const uuid &acl_id, const string &vrf_name,
const std::vector<VnIpam> &ipam,
const VnData::VnIpamDataMap &vn_ipam_data,
const VnData::VnIpamDataMap &vn_ipam_data, int vn_id,
int vxlan_id, bool admin_state, bool enable_rpf,
bool flood_unknown_unicast) {
DBRequest req;
VnKey *key = new VnKey(vn_uuid);
VnData *data = new VnData(agent(), name, acl_id, vrf_name, nil_uuid(),
nil_uuid(), ipam, vn_ipam_data,
vxlan_id, vxlan_id, true, true,
vn_id, vxlan_id, true, true,
admin_state, enable_rpf,
flood_unknown_unicast);

Expand Down
4 changes: 2 additions & 2 deletions src/vnsw/agent/oper/vn.h
Expand Up @@ -229,8 +229,8 @@ class VnTable : public AgentOperDBTable {

void AddVn(const uuid &vn_uuid, const string &name, const uuid &acl_id,
const string &vrf_name, const std::vector<VnIpam> &ipam,
const VnData::VnIpamDataMap &vn_ipam_data, int vxlan_id,
bool admin_state, bool enable_rpf,
const VnData::VnIpamDataMap &vn_ipam_data, int vn_id,
int vxlan_id, bool admin_state, bool enable_rpf,
bool flood_unknown_unicast);
void DelVn(const uuid &vn_uuid);
VnEntry *Find(const uuid &vn_uuid);
Expand Down
Expand Up @@ -41,18 +41,23 @@ MulticastMacLocalEntry::MulticastMacLocalEntry(MulticastMacLocalOvsdb *table,
logical_switch_(logical_switch) {
}

bool MulticastMacLocalEntry::Add() {
MulticastMacLocalOvsdb *table = static_cast<MulticastMacLocalOvsdb *>(table_);
OVSDB::VnOvsdbEntry *MulticastMacLocalEntry::GetVnEntry() const {
VnOvsdbObject *vn_object = table_->client_idl()->vn_ovsdb();
VnOvsdbEntry vn_key(vn_object, StringToUuid(logical_switch_name_));
VnOvsdbEntry *vn_entry =
static_cast<VnOvsdbEntry *>(vn_object->GetReference(&vn_key));
return vn_entry;
}

bool MulticastMacLocalEntry::Add() {
MulticastMacLocalOvsdb *table = static_cast<MulticastMacLocalOvsdb *>(table_);
OVSDB::VnOvsdbEntry *vn_entry = GetVnEntry();
// Take vrf reference to genrate withdraw/delete route request
vrf_ = vn_entry->vrf();
OVSDB_TRACE(Trace, "Adding multicast Route VN uuid " + logical_switch_name_);
vxlan_id_ = logical_switch_->vxlan_id();
table->peer()->AddOvsPeerMulticastRoute(vrf_.get(), vxlan_id_,
vn_entry->name(),
table_->client_idl()->tsn_ip(),
logical_switch_->tor_ip().to_v4());
return true;
Expand All @@ -79,15 +84,13 @@ bool MulticastMacLocalEntry::IsLess(const KSyncEntry& entry) const {
}

KSyncEntry *MulticastMacLocalEntry::UnresolvedReference() {
VnOvsdbObject *vn_object = table_->client_idl()->vn_ovsdb();
VnOvsdbEntry vn_key(vn_object, StringToUuid(logical_switch_name_));
VnOvsdbEntry *vn_entry =
static_cast<VnOvsdbEntry *>(vn_object->GetReference(&vn_key));
VnOvsdbEntry *vn_entry = GetVnEntry();
if (!vn_entry->IsResolved()) {
OVSDB_TRACE(Trace, "Skipping multicast route add " +
logical_switch_name_ + " due to unavailable VN ");
return vn_entry;
}

return NULL;
}

Expand Down
Expand Up @@ -10,6 +10,8 @@
class OvsPeer;

namespace OVSDB {
class VnOvsdbEntry;

class MulticastMacLocalOvsdb : public OvsdbObject {
public:
MulticastMacLocalOvsdb(OvsdbClientIdl *idl, OvsPeer *peer);
Expand Down Expand Up @@ -42,6 +44,7 @@ class MulticastMacLocalEntry : public OvsdbEntry {
const std::string &logical_switch_name() const;
const LogicalSwitchEntry *logical_switch() {return logical_switch_;}
const uint32_t vxlan_id() const {return vxlan_id_;}
OVSDB::VnOvsdbEntry *GetVnEntry() const;

private:
friend class MulticastMacLocalOvsdb;
Expand Down
Expand Up @@ -84,11 +84,12 @@ void OvsPeer::DeleteOvsRoute(VrfEntry *vrf, uint32_t vxlan_id,

void OvsPeer::AddOvsPeerMulticastRoute(const VrfEntry *vrf,
uint32_t vxlan_id,
const std::string &vn_name,
const Ip4Address &tsn_ip,
const Ip4Address &tor_ip) {
BridgeAgentRouteTable *table = static_cast<BridgeAgentRouteTable *>
(vrf->GetBridgeRouteTable());
table->AddOvsPeerMulticastRoute(this, vxlan_id, tsn_ip, tor_ip);
table->AddOvsPeerMulticastRoute(this, vxlan_id, vn_name, tsn_ip, tor_ip);
}

void OvsPeer::DeleteOvsPeerMulticastRoute(const VrfEntry *vrf,
Expand Down
Expand Up @@ -23,6 +23,7 @@ class OvsPeer : public Peer {
void DeleteOvsRoute(VrfEntry *vrf, uint32_t vxlan, const MacAddress &mac);
void AddOvsPeerMulticastRoute(const VrfEntry *vrf,
uint32_t vxlan_id,
const std::string &vn_name_,
const Ip4Address &tsn_ip,
const Ip4Address &tor_ip);
void DeleteOvsPeerMulticastRoute(const VrfEntry *vrf, uint32_t vxlan_id);
Expand Down
1 change: 1 addition & 0 deletions src/vnsw/agent/ovs_tor_agent/ovsdb_client/test/SConscript
Expand Up @@ -53,6 +53,7 @@ disabled_ovsdb_suite = []
test_ovs_route = AgentEnv.MakeTestCmd(env, 'test_ovs_route', flaky_agent_suite)
test_ovs_route = AgentEnv.MakeTestCmd(env, 'test_ovs_base', agent_suite)
test_ovs_route = AgentEnv.MakeTestCmd(env, 'test_ovs_logical_switch', agent_suite)
AgentEnv.MakeTestCmd(env, 'test_ovs_multicast_local', agent_suite)
AgentEnv.MakeTestCmd(env, 'test_ovs_unicast_remote', agent_suite)
AgentEnv.MakeTestCmd(env, 'test_ovs_unicast_local', agent_suite)
AgentEnv.MakeTestCmd(env, 'test_ovs_vlan_port', agent_suite)
Expand Down
@@ -0,0 +1,188 @@
/*
* Copyright (c) 2015 Juniper Networks, Inc. All rights reserved.
*/

#include "base/os.h"
#include "testing/gunit.h"

#include <base/logging.h>
#include <io/event_manager.h>
#include <io/test/event_manager_test.h>
#include <tbb/task.h>
#include <base/task.h>

#include <cmn/agent_cmn.h>

#include "cfg/cfg_init.h"
#include "cfg/cfg_interface.h"
#include "pkt/pkt_init.h"
#include "services/services_init.h"
#include "vrouter/ksync/ksync_init.h"
#include "oper/interface_common.h"
#include "oper/nexthop.h"
#include "oper/tunnel_nh.h"
#include "route/route.h"
#include "oper/vrf.h"
#include "oper/mpls.h"
#include "oper/vm.h"
#include "oper/vn.h"
#include "oper/physical_device_vn.h"
#include "filter/acl.h"
#include "openstack/instance_service_server.h"
#include "test_cmn_util.h"
#include "vr_types.h"

#include "openstack/instance_service_server.h"
#include "xmpp/xmpp_init.h"
#include "xmpp/test/xmpp_test_util.h"
#include "vr_types.h"
#include "control_node_mock.h"
#include "xml/xml_pugi.h"
#include "controller/controller_peer.h"
#include "controller/controller_export.h"
#include "controller/controller_vrf_export.h"

#include "ovs_tor_agent/ovsdb_client/ovsdb_route_peer.h"
#include "ovs_tor_agent/ovsdb_client/physical_switch_ovsdb.h"
#include "ovs_tor_agent/ovsdb_client/logical_switch_ovsdb.h"
#include "ovs_tor_agent/ovsdb_client/physical_port_ovsdb.h"
#include "test_ovs_agent_init.h"
#include "test-xml/test_xml.h"
#include "test-xml/test_xml_oper.h"
#include "test_xml_physical_device.h"
#include "test_xml_ovsdb.h"

#include <ovsdb_types.h>

using namespace pugi;
using namespace OVSDB;

EventManager evm1;
ServerThread *thread1;
test::ControlNodeMock *bgp_peer1;

EventManager evm2;
ServerThread *thread2;
test::ControlNodeMock *bgp_peer2;

void RouterIdDepInit(Agent *agent) {
Agent::GetInstance()->controller()->Connect();
}

class OvsBaseTest : public ::testing::Test {
protected:
OvsBaseTest() {
}

virtual void SetUp() {
agent_ = Agent::GetInstance();
init_ = static_cast<TestOvsAgentInit *>(client->agent_init());
peer_manager_ = init_->ovs_peer_manager();
WAIT_FOR(100, 10000,
(tcp_session_ = static_cast<OvsdbClientTcpSession *>
(init_->ovsdb_client()->NextSession(NULL))) != NULL);
WAIT_FOR(100, 10000,
(tcp_session_->client_idl() != NULL));
}

virtual void TearDown() {
}

void AddPhysicalDeviceVn(int dev_id, int vn_id) {
DBRequest req(DBRequest::DB_ENTRY_ADD_CHANGE);
req.key.reset(new PhysicalDeviceVnKey(MakeUuid(dev_id),
MakeUuid(vn_id)));
agent_->physical_device_vn_table()->Enqueue(&req);
PhysicalDeviceVn key(MakeUuid(dev_id), MakeUuid(vn_id));
WAIT_FOR(100, 10000,
(agent_->physical_device_vn_table()->Find(&key, false) != NULL));
}

void DelPhysicalDeviceVn(int dev_id, int vn_id) {
DBRequest req(DBRequest::DB_ENTRY_DELETE);
req.key.reset(new PhysicalDeviceVnKey(MakeUuid(dev_id),
MakeUuid(vn_id)));
agent_->physical_device_vn_table()->Enqueue(&req);
PhysicalDeviceVn key(MakeUuid(dev_id), MakeUuid(vn_id));
WAIT_FOR(100, 10000,
(agent_->physical_device_vn_table()->Find(&key, false) == NULL));
}

Agent *agent_;
TestOvsAgentInit *init_;
OvsPeerManager *peer_manager_;
OvsdbClientTcpSession *tcp_session_;
};

TEST_F(OvsBaseTest, BasicOvsdb) {
WAIT_FOR(100, 10000, (tcp_session_->status() == string("Established")));
}

TEST_F(OvsBaseTest, MulticastLocalBasic) {
AgentUtXmlTest
test("controller/src/vnsw/agent/ovs_tor_agent/ovsdb_client/test/xml/multicast-local-base.xml");
// set current session in test context
OvsdbTestSetSessionContext(tcp_session_);
AgentUtXmlOperInit(&test);
AgentUtXmlPhysicalDeviceInit(&test);
AgentUtXmlOvsdbInit(&test);
if (test.Load() == true) {
test.ReadXml();
string str;
test.ToString(&str);
cout << str << endl;
test.Run();
}
}

TEST_F(OvsBaseTest, MulticastLocal_add_mcroute_without_vrf_vn_link_present) {
//Add vrf
VrfAddReq("vrf1");
WAIT_FOR(100, 10000, (VrfGet("vrf1", false) != NULL));
//Add VN
VnAddReq(1, "vn1", "vrf1");
WAIT_FOR(100, 10000, (VnGet(1) != NULL));
//Add device
DBRequest device_req(DBRequest::DB_ENTRY_ADD_CHANGE);
device_req.key.reset(new PhysicalDeviceKey(MakeUuid(1)));
device_req.data.reset(new PhysicalDeviceData(agent_, "test-router",
"test-router", "",
Ip4Address::from_string("1.1.1.1"),
Ip4Address::from_string("2.2.2.2"),
"OVS", NULL));
agent_->physical_device_table()->Enqueue(&device_req);
WAIT_FOR(100, 10000,
(agent_->physical_device_table()->Find(MakeUuid(1)) != NULL));
//Add device_vn
AddPhysicalDeviceVn(1, 1);

//Initialization done, now delete VRF VN link and then update VXLAN id in
//VN.
TestClient::WaitForIdle();
VxLanNetworkIdentifierMode(true);
TestClient::WaitForIdle();

//Delete
DelPhysicalDeviceVn(1, 1);
DBRequest del_dev_req(DBRequest::DB_ENTRY_DELETE);
del_dev_req.key.reset(new PhysicalDeviceVnKey(MakeUuid(1),
MakeUuid(1)));
agent_->physical_device_table()->Enqueue(&del_dev_req);
WAIT_FOR(1000, 10000,
(agent_->physical_device_table()->
Find(MakeUuid(1)) == NULL));
VrfDelReq("vrf1");
VnDelReq(1);
WAIT_FOR(1000, 10000, (VrfGet("vrf1", true) != NULL));
WAIT_FOR(1000, 10000, (VnGet(1) != NULL));
}

int main(int argc, char *argv[]) {
GETUSERARGS();
// override with true to initialize ovsdb server and client
ksync_init = true;
client = OvsTestInit(init_file, ksync_init);
int ret = RUN_ALL_TESTS();
TestShutdown();
return ret;
}

0 comments on commit a904a76

Please sign in to comment.