Skip to content

Commit

Permalink
Remove systemlog messages generated for every route add/delete
Browse files Browse the repository at this point in the history
A collector systemlog message for every route add/delete will result in
overloading of controller with route logs. Removed the sytemlog for
route messages and made them trace messages

Change-Id: I052b8b60a58c709e6f9fc42a08217bccf651d260
Closes-Bug: #1563773
  • Loading branch information
praveenkv committed Mar 30, 2016
1 parent 73a4142 commit daf69db
Show file tree
Hide file tree
Showing 7 changed files with 95 additions and 22 deletions.
56 changes: 56 additions & 0 deletions src/vnsw/agent/controller/controller_route_path.cc
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,27 @@ bool CheckPeerValidity(const AgentXmppChannel *channel,
return false;
}

// Error string for invalid-peer
std::string GetInvalidPeerMsg(const std::string &type,
const AgentXmppChannel *channel,
uint64_t sequence_number) {
if (sequence_number == ControllerPeerPath::kInvalidPeerIdentifier) {
return type + " : Invalid sequence number";
}

assert(channel);
if (channel->bgp_peer_id() == NULL) {
return type + " : bgp-peer-id NULL";
}

if (sequence_number != channel->unicast_sequence_number()) {
return type + " : sequence number mis-match";
}

return type + " : Unexpected error";
}


ControllerPeerPath::ControllerPeerPath(const Peer *peer) :
AgentRouteData(false), peer_(peer) {
if ((peer != NULL) && (peer->GetType() == Peer::BGP_PEER) ) {
Expand All @@ -57,6 +78,11 @@ ControllerPeerPath::ControllerPeerPath(const Peer *peer) :
}
}

string ControllerEcmpRoute::PeerInvalidMsg(const AgentRouteKey *key) const {
return GetInvalidPeerMsg("ControllerEcmpRoute", channel(),
sequence_number());
}

bool ControllerEcmpRoute::IsPeerValid(const AgentRouteKey *key) const {
return CheckPeerValidity(channel(), sequence_number());
}
Expand Down Expand Up @@ -101,6 +127,10 @@ ControllerVmRoute *ControllerVmRoute::MakeControllerVmRoute(const Peer *peer,
return data;
}

string ControllerVmRoute::PeerInvalidMsg(const AgentRouteKey *key) const {
return GetInvalidPeerMsg("ControllerVmRoute", channel(), sequence_number());
}

bool ControllerVmRoute::IsPeerValid(const AgentRouteKey *key) const {
return CheckPeerValidity(channel(), sequence_number());
}
Expand Down Expand Up @@ -255,6 +285,11 @@ ControllerLocalVmRoute::ControllerLocalVmRoute(const VmInterfaceKey &intf,
path_preference, Ip4Address(0)),
sequence_number_(sequence_number), channel_(channel) { }

string ControllerLocalVmRoute::PeerInvalidMsg(const AgentRouteKey *key) const {
return GetInvalidPeerMsg("ControllerLocalVmRoute", channel_,
sequence_number_);
}

bool ControllerLocalVmRoute::IsPeerValid(const AgentRouteKey *key) const {
return CheckPeerValidity(channel_, sequence_number_);
}
Expand All @@ -270,6 +305,11 @@ ControllerVlanNhRoute::ControllerVlanNhRoute(const VmInterfaceKey &intf,
VlanNhRoute(intf, tag, label, dest_vn_name, sg_list, path_preference),
sequence_number_(sequence_number), channel_(channel) { }

string ControllerVlanNhRoute::PeerInvalidMsg(const AgentRouteKey *key) const {
return GetInvalidPeerMsg("ControllerVlanNhRoute", channel_,
sequence_number_);
}

bool ControllerVlanNhRoute::IsPeerValid(const AgentRouteKey *key) const {
return CheckPeerValidity(channel_, sequence_number_);
}
Expand All @@ -283,6 +323,12 @@ ControllerInetInterfaceRoute::ControllerInetInterfaceRoute(const InetInterfaceKe
InetInterfaceRoute(intf, label, tunnel_bmap, dest_vn_name),
sequence_number_(sequence_number), channel_(channel) { }

string ControllerInetInterfaceRoute::PeerInvalidMsg
(const AgentRouteKey *key) const {
return GetInvalidPeerMsg("ConrollerInetInterfaceRoute", channel_,
sequence_number_);
}

bool ControllerInetInterfaceRoute::IsPeerValid(const AgentRouteKey *key) const {
return CheckPeerValidity(channel_, sequence_number_);
}
Expand Down Expand Up @@ -355,6 +401,10 @@ bool ClonedLocalPath::AddChangePath(Agent *agent, AgentPath *path,
return ret;
}

string ClonedLocalPath::PeerInvalidMsg(const AgentRouteKey *key) const {
return GetInvalidPeerMsg("ClonedLocalPath", channel_, sequence_number_);
}

bool ClonedLocalPath::IsPeerValid(const AgentRouteKey *key) const {
return CheckPeerValidity(channel_, sequence_number_);
}
Expand All @@ -370,6 +420,12 @@ ControllerMulticastRoute::ControllerMulticastRoute(const string &vn_name,
MulticastRoute(vn_name, label, vxlan_id, tunnel_type, nh_req, comp_nh_type),
sequence_number_(sequence_number), channel_(channel) { }

string ControllerMulticastRoute::PeerInvalidMsg
(const AgentRouteKey *key) const {
return GetInvalidPeerMsg("ControllerMulticastRoute", channel_,
sequence_number_);
}

bool ControllerMulticastRoute::IsPeerValid(const AgentRouteKey *key) const {
return CheckPeerValidity(channel_, sequence_number_);
}
7 changes: 7 additions & 0 deletions src/vnsw/agent/controller/controller_route_path.h
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ class ControllerVmRoute : public ControllerPeerPath {
const AgentRoute *rt);
virtual bool UpdateRoute(AgentRoute *route);
virtual string ToString() const {return "remote VM";}
virtual std::string PeerInvalidMsg(const AgentRouteKey *key) const;
virtual bool IsPeerValid(const AgentRouteKey *key) const;
const SecurityGroupList &sg_list() const {return sg_list_;}
static ControllerVmRoute *MakeControllerVmRoute(const Peer *peer,
Expand Down Expand Up @@ -138,6 +139,7 @@ class ControllerEcmpRoute : public ControllerPeerPath {
virtual bool AddChangePath(Agent *agent, AgentPath *path,
const AgentRoute *);
virtual string ToString() const {return "inet4 ecmp";}
virtual std::string PeerInvalidMsg(const AgentRouteKey *key) const;
virtual bool IsPeerValid(const AgentRouteKey *key) const;

private:
Expand Down Expand Up @@ -169,6 +171,7 @@ class ControllerLocalVmRoute : public LocalVmRoute {
uint64_t sequence_number,
const AgentXmppChannel *channel);
virtual ~ControllerLocalVmRoute() { }
virtual std::string PeerInvalidMsg(const AgentRouteKey *key) const;
virtual bool IsPeerValid(const AgentRouteKey *key) const;

private:
Expand All @@ -189,6 +192,7 @@ class ControllerInetInterfaceRoute : public InetInterfaceRoute {
uint64_t sequence_number,
const AgentXmppChannel *channel);
virtual ~ControllerInetInterfaceRoute() { }
virtual std::string PeerInvalidMsg(const AgentRouteKey *key) const;
virtual bool IsPeerValid(const AgentRouteKey *key) const;

private:
Expand All @@ -211,6 +215,7 @@ class ControllerVlanNhRoute : public VlanNhRoute {
uint64_t sequence_number,
const AgentXmppChannel *channel);
virtual ~ControllerVlanNhRoute() { }
virtual std::string PeerInvalidMsg(const AgentRouteKey *key) const;
virtual bool IsPeerValid(const AgentRouteKey *key) const;

private:
Expand All @@ -236,6 +241,7 @@ class ClonedLocalPath : public AgentRouteData {
AgentRouteData(false), sequence_number_(seq),
channel_(channel), mpls_label_(label), vn_(vn), sg_list_(sg_list) {}
virtual ~ClonedLocalPath() {}
virtual std::string PeerInvalidMsg(const AgentRouteKey *key) const;
virtual bool IsPeerValid(const AgentRouteKey *key) const;
virtual bool AddChangePath(Agent *agent, AgentPath *path,
const AgentRoute *rt);
Expand All @@ -262,6 +268,7 @@ class ControllerMulticastRoute : public MulticastRoute {
uint64_t sequence_number,
const AgentXmppChannel *channel);
virtual ~ControllerMulticastRoute() { }
virtual std::string PeerInvalidMsg(const AgentRouteKey *key) const;
virtual bool IsPeerValid(const AgentRouteKey *key) const;

private:
Expand Down
2 changes: 1 addition & 1 deletion src/vnsw/agent/oper/agent.sandesh
Original file line number Diff line number Diff line change
Expand Up @@ -1066,7 +1066,7 @@ traceobject sandesh OperNextHop {
1: NextHopObjectLogInfo nh;
}

systemlog sandesh AgentRouteLog {
trace sandesh AgentRouteLog {
1: string message;
2: string ip;
3: "in VRF";
Expand Down
30 changes: 16 additions & 14 deletions src/vnsw/agent/oper/agent_route.cc
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@ using namespace boost::asio;
SandeshTraceBufferPtr AgentDBwalkTraceBuf(SandeshTraceBufferCreate(
AGENT_DBWALK_TRACE_BUF, 1000));

SandeshTraceBufferPtr AgentRouteTraceBuf(SandeshTraceBufferCreate(
AGENT_ROUTE_TRACE_BUF, 5000));

class AgentRouteTable::DeleteActor : public LifetimeActor {
public:
DeleteActor(AgentRouteTable *rt_table) :
Expand Down Expand Up @@ -237,7 +240,7 @@ void AgentRouteTable::DeletePathFromPeer(DBTablePartBase *part,

RouteInfo rt_info;
rt->FillTrace(rt_info, AgentRoute::DELETE_PATH, path);
OPER_TRACE(Route, rt_info);
ROUTE_OPER_TRACE(Route, rt_info);

if (path == NULL) {
return;
Expand All @@ -259,7 +262,7 @@ void AgentRouteTable::DeletePathFromPeer(DBTablePartBase *part,
if (rt->GetActivePath() == NULL) {
RouteInfo rt_info_del;
rt->FillTrace(rt_info_del, AgentRoute::DELETE, NULL);
OPER_TRACE(Route, rt_info_del);
ROUTE_OPER_TRACE(Route, rt_info_del);
PreRouteDelete(rt);
RemoveUnresolvedRoute(rt);
rt->UpdateDependantRoutes();
Expand Down Expand Up @@ -333,10 +336,9 @@ void AgentRouteTable::Input(DBTablePartition *part, DBClient *client,

if (data) {
if (data->IsPeerValid(key) == false) {
AGENT_ROUTE_LOG("Invalid/Inactive Peer ",
key->ToString(),
vrf_name(),
"");
AGENT_ROUTE_LOG("Route operation ignored. Invalid/Inactive Peer ",
key->ToString(), vrf_name(),
data->InvalidPeerMsg(key));
return;
}
} else {
Expand Down Expand Up @@ -388,10 +390,8 @@ void AgentRouteTable::Input(DBTablePartition *part, DBClient *client,
ProcessAdd(rt);
RouteInfo rt_info;
rt->FillTrace(rt_info, AgentRoute::ADD, NULL);
OPER_TRACE(Route, rt_info);
ROUTE_OPER_TRACE(Route, rt_info);
route_added = true;
AGENT_ROUTE_LOG("Added route", rt->ToString(), vrf_name(),
GETPEERNAME(key->peer()));
} else {
// RT present. Check if path is also present by peer
path = rt->FindPathUsingKeyData(key, data);
Expand All @@ -411,9 +411,7 @@ void AgentRouteTable::Input(DBTablePartition *part, DBClient *client,

RouteInfo rt_info;
rt->FillTrace(rt_info, AgentRoute::ADD_PATH, path);
OPER_TRACE(Route, rt_info);
AGENT_ROUTE_LOG("Path add", rt->ToString(), vrf_name(),
GETPEERNAME(key->peer()));
ROUTE_OPER_TRACE(Route, rt_info);
} else {
// Let path know of route change and update itself
path->set_is_stale(false);
Expand All @@ -428,7 +426,7 @@ void AgentRouteTable::Input(DBTablePartition *part, DBClient *client,
RouteInfo rt_info;

rt->FillTrace(rt_info, AgentRoute::CHANGE_PATH, path);
OPER_TRACE(Route, rt_info);
ROUTE_OPER_TRACE(Route, rt_info);
}

if (path->RouteNeedsSync())
Expand Down Expand Up @@ -746,7 +744,7 @@ void AgentRouteTable::StalePathFromPeer(DBTablePartBase *part, AgentRoute *rt,

RouteInfo rt_info;
rt->FillTrace(rt_info, AgentRoute::STALE_PATH, path);
OPER_TRACE(Route, rt_info);
ROUTE_OPER_TRACE(Route, rt_info);

if (path == NULL) {
return;
Expand Down Expand Up @@ -777,6 +775,10 @@ bool AgentRouteData::IsPeerValid(const AgentRouteKey *key) const {
return true;
}

std::string AgentRouteData::InvalidPeerMsg(const AgentRouteKey *key) const {
return "AgentRouteData: Unknown Reason";
}

AgentPath *AgentRouteData::CreateAgentPath(const Peer *peer,
AgentRoute *rt) const {
return (new AgentPath(peer, rt));
Expand Down
14 changes: 11 additions & 3 deletions src/vnsw/agent/oper/agent_route.h
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ struct AgentRouteData : public AgentData {
virtual bool AddChangePath(Agent *agent, AgentPath *path,
const AgentRoute *rt) = 0;
virtual bool IsPeerValid(const AgentRouteKey *key) const;
virtual std::string InvalidPeerMsg(const AgentRouteKey *key) const;
virtual bool UpdateRoute(AgentRoute *rt) {return false;}

bool is_multicast() const {return is_multicast_;}
Expand Down Expand Up @@ -295,18 +296,25 @@ class AgentRoute : public Route {
};

#define AGENT_DBWALK_TRACE_BUF "AgentDBwalkTrace"
#define AGENT_ROUTE_TRACE_BUF "OperRouteTrace"

extern SandeshTraceBufferPtr AgentDBwalkTraceBuf;
extern SandeshTraceBufferPtr AgentRouteTraceBuf;

#define AGENT_DBWALK_TRACE(obj, ...) do { \
obj::TraceMsg(AgentDBwalkTraceBuf, __FILE__, __LINE__, ##__VA_ARGS__); \
} while (0);

#define GETPEERNAME(peer) (peer)? peer->GetName() : ""
#define AGENT_ROUTE_LOG(oper, route, vrf, peer_name)\
#define AGENT_ROUTE_LOG(msg, route, vrf, peer_info)\
do {\
AgentRouteLog::Send("Agent", SandeshLevel::SYS_INFO, __FILE__, __LINE__,\
oper, route, vrf, peer_name);\
AgentRouteLog::TraceMsg(AgentRouteTraceBuf, __FILE__, __LINE__, msg, route,\
vrf, peer_info);\
} while(false);\

#define ROUTE_OPER_TRACE(type, rt_info)\
do {\
OperRoute::TraceMsg(AgentRouteTraceBuf, __FILE__, __LINE__, rt_info);\
} while(false);\

#endif
6 changes: 3 additions & 3 deletions src/vnsw/agent/oper/inet_unicast_route.cc
Original file line number Diff line number Diff line change
Expand Up @@ -431,7 +431,7 @@ AgentPath *InetUnicastRouteEntry::AllocateEcmpPath(Agent *agent,

RouteInfo rt_info;
FillTrace(rt_info, AgentRoute::CHANGE_PATH, path);
OPER_TRACE(Route, rt_info);
ROUTE_OPER_TRACE(Route, rt_info);
AGENT_ROUTE_LOG("Path change", ToString(), vrf()->GetName(),
GETPEERNAME(agent->ecmp_peer()));

Expand Down Expand Up @@ -649,7 +649,7 @@ void InetUnicastRouteEntry::AppendEcmpPath(Agent *agent, AgentPath *path) {

RouteInfo rt_info;
FillTrace(rt_info, AgentRoute::CHANGE_PATH, path);
OPER_TRACE(Route, rt_info);
ROUTE_OPER_TRACE(Route, rt_info);
AGENT_ROUTE_LOG("Path change", ToString(), vrf()->GetName(),
GETPEERNAME(agent->ecmp_peer()));
}
Expand Down Expand Up @@ -688,7 +688,7 @@ void InetUnicastRouteEntry::DeleteComponentNH(Agent *agent, AgentPath *path) {

RouteInfo rt_info;
FillTrace(rt_info, AgentRoute::CHANGE_PATH, path);
OPER_TRACE(Route, rt_info);
ROUTE_OPER_TRACE(Route, rt_info);
AGENT_ROUTE_LOG("Path change", ToString(), vrf()->GetName(),
GETPEERNAME(agent->ecmp_peer()));
}
Expand Down
2 changes: 1 addition & 1 deletion src/vnsw/agent/test-xml/test_xml.cc
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ bool AgentUtXmlTest::ReadXml() {
AgentUtXmlTestCase *test = new AgentUtXmlTestCase(attr.value(),
node, this);
attr = node.attribute("verbose");
bool verbose;
bool verbose = false;
if (!attr) {
verbose = false;
} else {
Expand Down

0 comments on commit daf69db

Please sign in to comment.