Skip to content

Commit

Permalink
Reverting the change done to retain NH ids upon restart
Browse files Browse the repository at this point in the history 
There are cases where the same NH id is being allocated to multiple composite
NHs, resulting in crash during free. This change is not really used
and hence reverting the same. Can be added back in the later with
necessary changes.

Change-Id: I357c1c17c98b08f75622dfc5c6ca89096a32f96b
closes-bug: #1764506
  • Loading branch information
@esnagendra
esnagendra committed Sep 7, 2018
1 parent e59ed76 commit 0ac3c12
Show file tree
Hide file tree
Showing 13 changed files with 11 additions and 764 deletions.
2 changes: 1 addition & 1 deletion src/vnsw/agent/init/agent_init.cc
View file
Expand Up @@ -40,9 +40,9 @@ AgentInit::~AgentInit() {
controller_.reset();
cfg_.reset();
oper_.reset();
resource_manager_.reset();
agent_->db()->ClearFactoryRegistry();
agent_.reset();
resource_manager_.reset();

scheduler->Terminate();
}
Expand Down
72 changes: 5 additions & 67 deletions src/vnsw/agent/oper/nexthop.cc
View file
Expand Up @@ -18,9 +18,6 @@
#include <oper/vrf.h>
#include <oper/agent_sandesh.h>
#include <oper/crypt_tunnel.h>
#include <resource_manager/resource_manager.h>
#include <resource_manager/resource_table.h>
#include <resource_manager/nexthop_index.h>

using namespace std;

Expand Down Expand Up @@ -92,19 +89,9 @@ void NextHop::SendObjectLog(const NextHopTable *table,
NextHop::~NextHop() {
if (id_ != kInvalidIndex) {
static_cast<NextHopTable *>(get_table())->FreeInterfaceId(id_);
id_ = kInvalidIndex;
}
}

void NextHopTable::FreeInterfaceId(size_t index) {
if (index <= 2) {
agent()->resource_manager()->ReleaseIndex(Resource::NEXTHOP_INDEX, index);
} else {
agent()->resource_manager()->Release(Resource::NEXTHOP_INDEX, index);
}
index_table_.Remove(index);
}

void NextHop::SetKey(const DBRequestKey *key) {
const NextHopKey *nh_key = static_cast<const NextHopKey *>(key);
type_ = nh_key->type_;
Expand Down Expand Up @@ -284,23 +271,16 @@ bool NextHop::NexthopToInterfacePolicy() const {
NextHopTable::NextHopTable(DB *db, const string &name) : AgentDBTable(db, name){
// nh-index 0 is reserved by vrouter. So, pre-allocate the first index so
// that nh added by agent use index 1 and above
Agent::GetInstance()->resource_manager()->
ReserveIndex(Resource::NEXTHOP_INDEX, 0);
Agent::GetInstance()->resource_manager()->
ReserveIndex(Resource::NEXTHOP_INDEX, 1);
uint32_t id = index_table_.InsertAtIndex(0, NULL);
index_table_.InsertAtIndex(1, NULL);
int id = index_table_.Insert(NULL);
assert(id == 0);
}

NextHopTable::~NextHopTable() {
FreeInterfaceId(0);
}
// Reserve the index for DiscardNHKey

uint32_t NextHopTable::ReserveIndex() {
agent()->resource_manager()->ReserveIndex(Resource::NEXTHOP_INDEX, 2);
index_table_.InsertAtIndex(NextHopTable::kRpfDiscardIndex, NULL);
return NextHopTable::kRpfDiscardIndex;
return index_table_.Insert(NULL);
}

std::auto_ptr<DBEntry> NextHopTable::AllocEntry(const DBRequestKey *k) const {
Expand All @@ -319,53 +299,12 @@ std::auto_ptr<DBEntry> NextHopTable::GetEntry(const DBRequestKey *key) const {
DBEntry *NextHopTable::Add(const DBRequest *req) {
const NextHopKey *key = static_cast<const NextHopKey *>(req->key.get());
NextHop *nh = AllocWithKey(key);
uint32_t index;

if (nh->CanAdd() == false) {
delete nh;
return NULL;
}
//parse the CompositeNHKey and fill the nh_id's & lables
if (nh->GetType() == NextHop::COMPOSITE) {
CompositeNH *cnh = static_cast<CompositeNH*>(nh);
std::vector<cnhid_label_map>nh_ids;
ComponentNHKeyList::const_iterator it =
cnh->component_nh_key_list().begin();
for (;it != cnh->component_nh_key_list().end(); it++) {
cnhid_label_map cnh_label = {0};
if ((*it) != NULL) {
CompositeNHKey *composite_nh_key =
static_cast<CompositeNHKey *>((*it)->nh_key()->Clone());
const NextHop *nexthop = static_cast<const NextHop *>
(NextHopTable::GetInstance()->FindActiveEntry(composite_nh_key));
if (nexthop) {
cnh_label.nh_id = nexthop->id();
cnh_label.label = (*it)->label();
}
delete composite_nh_key;
}
nh_ids.push_back(cnh_label);
}
ResourceManager::KeyPtr rkey(new NHIndexResourceKey
(agent()->resource_manager(),
NextHop::COMPOSITE,
(cnh)->composite_nh_type(),
nh_ids, nh->PolicyEnabled(),
cnh->vrf()->GetName()));
index = ((static_cast<IndexResourceData *>
(agent()->resource_manager()->Allocate(rkey).get()))->index());
} else if (nh->GetType() != NextHop::DISCARD) {
ResourceManager::KeyPtr rkey(new NHIndexResourceKey
(agent()->resource_manager(),
nh->GetType(),
key->Clone()));
index = ((static_cast<IndexResourceData *>
(agent()->resource_manager()->Allocate(rkey).get()))->index());
} else {
// will be a discard index
index = NextHopTable::kRpfDiscardIndex;
}
nh->set_id(index);
index_table_.InsertAtIndex(index, nh);
nh->set_id(index_table_.Insert(nh));
nh->Add(agent(), req);
nh->SendObjectLog(this, AgentLogEvent::ADD);
return static_cast<DBEntry *>(nh);
Expand Down Expand Up @@ -444,7 +383,6 @@ NextHop *ArpNHKey::AllocEntry() const {
return new ArpNH(vrf, dip_);
}


bool ArpNH::CanAdd() const {
if (vrf_ == NULL) {
LOG(ERROR, "Invalid VRF in ArpNH. Skip Add");
Expand Down
40 changes: 4 additions & 36 deletions src/vnsw/agent/oper/nexthop.h
View file
Expand Up @@ -628,14 +628,6 @@ class ReceiveNHKey : public NextHopKey {
virtual NextHopKey *Clone() const {
return new ReceiveNHKey(intf_key_->Clone(), policy_);
}
const InterfaceKey* intf_key() const {return intf_key_.get();}
virtual bool NextHopKeyIsLess(const NextHopKey &rhs) const {
const ReceiveNHKey &key = static_cast<const ReceiveNHKey&>(rhs);
if (intf_key_->IsEqual(*key.intf_key_.get()) == false) {
return intf_key_->IsLess(*key.intf_key_.get());
}
return false;
}

private:
friend class ReceiveNH;
Expand Down Expand Up @@ -707,15 +699,7 @@ class ResolveNHKey : public NextHopKey {

virtual NextHop *AllocEntry() const;
virtual NextHopKey *Clone() const {
return new ResolveNHKey(intf_key_.get(), policy_);
}
const InterfaceKey* intf_key() const {return intf_key_.get();}
virtual bool NextHopKeyIsLess(const NextHopKey &rhs) const {
const ResolveNHKey &key = static_cast<const ResolveNHKey&>(rhs);
if (intf_key_->IsEqual(*key.intf_key_.get()) == false) {
return intf_key_->IsLess(*key.intf_key_.get());
}
return false;
return new ResolveNHKey(intf_key_->Clone(), policy_);
}
private:
friend class ResolveNH;
Expand Down Expand Up @@ -785,15 +769,6 @@ class ArpNHKey : public NextHopKey {
virtual NextHopKey *Clone() const {
return new ArpNHKey(vrf_key_.name_, dip_, policy_);
}
const Ip4Address& dip() const { return dip_; }
const string& vrf_name() const { return vrf_key_.name_; }
virtual bool NextHopKeyIsLess(const NextHopKey &rhs) const {
const ArpNHKey &key = static_cast<const ArpNHKey &>(rhs);
if (vrf_key_.IsEqual(key.vrf_key_) == false) {
return vrf_key_.IsLess(key.vrf_key_);
}
return dip_ < key.dip_;
}
private:
friend class ArpNH;
VrfKey vrf_key_;
Expand Down Expand Up @@ -915,11 +890,6 @@ class TunnelNHKey : public NextHopKey {
const Ip4Address dip() const {
return dip_;
}
const Ip4Address sip() const {
return sip_;
}
const string& vrf_name() const { return vrf_key_.name_; }
const uint16_t tunnel_type() const {return tunnel_type_.GetType();}
private:
friend class TunnelNH;
VrfKey vrf_key_;
Expand Down Expand Up @@ -968,8 +938,6 @@ class PBBNHKey : public NextHopKey {
const MacAddress dest_bmac() const {
return dest_bmac_;
}
const uint32_t isid() const {return isid_;}
const string& vrf_name() const { return vrf_key_.name_;}
private:
friend class PBBNH;
VrfKey vrf_key_;
Expand Down Expand Up @@ -1053,6 +1021,7 @@ struct InterfaceNHFlags {
VXLAN_ROUTING = 16
};
};

class InterfaceNHKey : public NextHopKey {
public:
InterfaceNHKey(InterfaceKey *intf, bool policy, uint8_t flags,
Expand Down Expand Up @@ -1262,7 +1231,6 @@ class VrfNHKey : public NextHopKey {
return new VrfNHKey(vrf_key_.name_, policy_, bridge_nh_);
}
const std::string &GetVrfName() const { return vrf_key_.name_; }
const bool GetPolicy() const { return policy_; }
const bool &GetBridgeNh() const { return bridge_nh_; }

private:
Expand Down Expand Up @@ -1578,7 +1546,7 @@ class CompositeNHKey : public NextHopKey {
void CreateTunnelNHReq(Agent *agent);
void ChangeTunnelType(TunnelType::Type tunnel_type);
COMPOSITETYPE composite_nh_type() const {return composite_nh_type_;}
const string& vrf_name() const {return vrf_key_.name_;}

void ReplaceLocalNexthop(const ComponentNHKeyList &new_comp_nh);
private:
friend class CompositeNH;
Expand Down Expand Up @@ -1796,7 +1764,7 @@ class NextHopTable : public AgentDBTable {
void set_l2_receive_nh(NextHop *nh) { l2_receive_nh_ = nh; }
NextHop *l2_receive_nh() const {return l2_receive_nh_;}
// NextHop index managing routines
void FreeInterfaceId(size_t index);
void FreeInterfaceId(size_t index) { index_table_.Remove(index); }
NextHop *FindNextHop(size_t index);
uint32_t ReserveIndex();

Expand Down
24 changes: 0 additions & 24 deletions src/vnsw/agent/oper/tunnel_nh.h
View file
Expand Up @@ -85,30 +85,6 @@ class MirrorNHKey : public NextHopKey {
virtual NextHopKey *Clone() const {
return new MirrorNHKey(vrf_key_.name_, sip_, sport_, dip_, dport_);
}
const IpAddress & sip() const {return sip_;}
const IpAddress & dip() const {return dip_;}
const uint32_t sport() const {return sport_;}
const uint32_t dport() const {return dport_;}
const string & vrf_name() const { return vrf_key_.name_ ; }
virtual bool NextHopKeyIsLess(const NextHopKey &rhs) const {
const MirrorNHKey &key = static_cast<const MirrorNHKey &>(rhs);
if (vrf_key_.IsEqual(key.vrf_key_) == false) {
return vrf_key_.IsLess(key.vrf_key_);
}
if (sip_ != key.sip_) {
return sip_ < key.sip_;
}

if (dip_ != key.dip_) {
return dip_ < key.dip_;
}

if (sport_ != key.sport_) {
return sport_ < key.sport_;
}

return dport_ < key.dport_;
}
private:
friend class MirrorNH;
VrfKey vrf_key_;
Expand Down
9 changes: 0 additions & 9 deletions src/vnsw/agent/resource_manager/README
View file
Expand Up @@ -91,12 +91,3 @@ fallback time period.

Hash value will be calculated for the file content. and appended to file name.
This Value will be used to validate while reading the file.

Restoring the Nexthop indexs
---------------------------
Maintained flat Sandesh structure to store all the nexthops
in to file based on the type of the Key nexthop key will be retrived.

Composite Nexthop Sandesh structure maintained separate
as the nexthop resource key is represented diffrent args. Composite next hop is
retrieved based on the Nhid to label maps parased form CompositeNextHopKey.
1 change: 0 additions & 1 deletion src/vnsw/agent/resource_manager/SConscript
View file
Expand Up @@ -23,7 +23,6 @@ libresource_manager = env.Library('resource_manager',
'qos_index.cc',
'bgp_as_service_index.cc',
'mirror_index.cc',
'nexthop_index.cc',
'resource_backup.cc',
except_env.Object('sandesh_map.cc'),
])
Expand Down

0 comments on commit 0ac3c12

Please sign in to comment.