Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

VNET/VRF Changes #632

Merged
merged 7 commits into from
Oct 10, 2018
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions orchagent/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ orchagent_SOURCES = \
vrforch.cpp \
countercheckorch.cpp \
vxlanorch.cpp \
vnetorch.cpp \
dtelorch.cpp \
flexcounterorch.cpp \
acltable.h \
Expand Down Expand Up @@ -74,6 +75,7 @@ orchagent_SOURCES = \
dtelorch.h \
countercheckorch.h \
vxlanorch.h \
vnetorch.h \
flexcounterorch.h \
$(top_srcdir)/warmrestart/warm_restart.cpp \
$(top_srcdir)/warmrestart/warm_restart.h
Expand Down
64 changes: 46 additions & 18 deletions orchagent/intfsorch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ extern BufferOrch *gBufferOrch;

const int intfsorch_pri = 35;

IntfsOrch::IntfsOrch(DBConnector *db, string tableName) :
Orch(db, tableName, intfsorch_pri)
IntfsOrch::IntfsOrch(DBConnector *db, string tableName, VRFOrch *vrf_orch) :
Orch(db, tableName, intfsorch_pri), m_vrfOrch(vrf_orch)
{
SWSS_LOG_ENTER();
}
Expand Down Expand Up @@ -113,7 +113,31 @@ void IntfsOrch::doTask(Consumer &consumer)

vector<string> keys = tokenize(kfvKey(t), ':');
string alias(keys[0]);
IpPrefix ip_prefix(kfvKey(t).substr(kfvKey(t).find(':')+1));
IpPrefix ip_prefix;
bool ip_prefix_in_key = false;

if (keys.size() > 1)
{
ip_prefix = kfvKey(t).substr(kfvKey(t).find(':')+1);
ip_prefix_in_key = true;
}

const vector<FieldValueTuple>& data = kfvFieldsValues(t);
string vrf_name = "", vnet_name = "";

for (auto idx : data)
{
const auto &field = fvField(idx);
const auto &value = fvValue(idx);
if (field == "vrf_name")
{
vrf_name = value;
}
else if (field == "vnet_name")
{
vnet_name = vrf_name = value;
}
}

if (alias == "eth0" || alias == "docker0")
{
Expand All @@ -126,7 +150,7 @@ void IntfsOrch::doTask(Consumer &consumer)
{
if (alias == "lo")
{
addIp2MeRoute(ip_prefix);
addIp2MeRoute(vrf_name, ip_prefix);
it = consumer.m_toSync.erase(it);
continue;
}
Expand All @@ -149,7 +173,7 @@ void IntfsOrch::doTask(Consumer &consumer)
auto it_intfs = m_syncdIntfses.find(alias);
if (it_intfs == m_syncdIntfses.end())
{
if (addRouterIntfs(port))
if (addRouterIntfs(vrf_name, port))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When the interface is enslaved in a vrf, the connected (subnet route) and receive (ip2me route) need to be removed from the old vrf. Is the expectation that SAI will do this automatically?

I don't see this PR taking care of that and I don't see #635 taking care of that either.

Please clarify whether it's been agreed that SAI will do this. If not, then either this PR or the one I mentioned above, need to take care of it, unless I've missed something.

{
IntfsEntry intfs_entry;
intfs_entry.ref_count = 0;
Expand All @@ -162,9 +186,9 @@ void IntfsOrch::doTask(Consumer &consumer)
}
}

if (m_syncdIntfses[alias].ip_addresses.count(ip_prefix))
if (!ip_prefix_in_key || m_syncdIntfses[alias].ip_addresses.count(ip_prefix))
{
/* Duplicate entry */
/* Request to create router interface, no prefix present or Duplicate entry */
it = consumer.m_toSync.erase(it);
continue;
}
Expand Down Expand Up @@ -198,7 +222,7 @@ void IntfsOrch::doTask(Consumer &consumer)
}

addSubnetRoute(port, ip_prefix);
addIp2MeRoute(ip_prefix);
addIp2MeRoute(vrf_name, ip_prefix);

if (port.m_type == Port::VLAN && ip_prefix.isV4())
{
Expand All @@ -212,7 +236,7 @@ void IntfsOrch::doTask(Consumer &consumer)
{
if (alias == "lo")
{
removeIp2MeRoute(ip_prefix);
removeIp2MeRoute(vrf_name, ip_prefix);
it = consumer.m_toSync.erase(it);
continue;
}
Expand All @@ -230,7 +254,7 @@ void IntfsOrch::doTask(Consumer &consumer)
if (m_syncdIntfses[alias].ip_addresses.count(ip_prefix))
{
removeSubnetRoute(port, ip_prefix);
removeIp2MeRoute(ip_prefix);
removeIp2MeRoute(vrf_name, ip_prefix);
if(port.m_type == Port::VLAN && ip_prefix.isV4())
{
removeDirectedBroadcast(port, ip_prefix.getBroadcastIp());
Expand Down Expand Up @@ -262,7 +286,7 @@ void IntfsOrch::doTask(Consumer &consumer)
}
}

bool IntfsOrch::addRouterIntfs(Port &port)
bool IntfsOrch::addRouterIntfs(string& vrf_name, Port &port)
{
SWSS_LOG_ENTER();

Expand All @@ -274,12 +298,14 @@ bool IntfsOrch::addRouterIntfs(Port &port)
return true;
}

sai_object_id_t vr_id = m_vrfOrch->getVRFid(vrf_name);

/* Create router interface if the router interface doesn't exist */
sai_attribute_t attr;
vector<sai_attribute_t> attrs;

attr.id = SAI_ROUTER_INTERFACE_ATTR_VIRTUAL_ROUTER_ID;
attr.value.oid = gVirtualRouterId;
attr.value.oid = vr_id;
attrs.push_back(attr);

attr.id = SAI_ROUTER_INTERFACE_ATTR_SRC_MAC_ADDRESS;
Expand Down Expand Up @@ -334,6 +360,8 @@ bool IntfsOrch::addRouterIntfs(Port &port)
throw runtime_error("Failed to create router interface.");
}

port.m_vr_id = vr_id;

gPortsOrch->setPort(port.m_alias, port);

SWSS_LOG_NOTICE("Create router interface %s MTU %u", port.m_alias.c_str(), port.m_mtu);
Expand Down Expand Up @@ -370,7 +398,7 @@ void IntfsOrch::addSubnetRoute(const Port &port, const IpPrefix &ip_prefix)
{
sai_route_entry_t unicast_route_entry;
unicast_route_entry.switch_id = gSwitchId;
unicast_route_entry.vr_id = gVirtualRouterId;
unicast_route_entry.vr_id = port.m_vr_id;
copy(unicast_route_entry.destination, ip_prefix);
subnet(unicast_route_entry.destination, unicast_route_entry.destination);

Expand Down Expand Up @@ -413,7 +441,7 @@ void IntfsOrch::removeSubnetRoute(const Port &port, const IpPrefix &ip_prefix)
{
sai_route_entry_t unicast_route_entry;
unicast_route_entry.switch_id = gSwitchId;
unicast_route_entry.vr_id = gVirtualRouterId;
unicast_route_entry.vr_id = port.m_vr_id;
copy(unicast_route_entry.destination, ip_prefix);
subnet(unicast_route_entry.destination, unicast_route_entry.destination);

Expand Down Expand Up @@ -441,11 +469,11 @@ void IntfsOrch::removeSubnetRoute(const Port &port, const IpPrefix &ip_prefix)
gRouteOrch->notifyNextHopChangeObservers(ip_prefix, IpAddresses(), false);
}

void IntfsOrch::addIp2MeRoute(const IpPrefix &ip_prefix)
void IntfsOrch::addIp2MeRoute(string &vrf_name, const IpPrefix &ip_prefix)
{
sai_route_entry_t unicast_route_entry;
unicast_route_entry.switch_id = gSwitchId;
unicast_route_entry.vr_id = gVirtualRouterId;
unicast_route_entry.vr_id = m_vrfOrch->getVRFid(vrf_name);;
prsunny marked this conversation as resolved.
Show resolved Hide resolved
copy(unicast_route_entry.destination, ip_prefix.getIp());

sai_attribute_t attr;
Expand Down Expand Up @@ -481,11 +509,11 @@ void IntfsOrch::addIp2MeRoute(const IpPrefix &ip_prefix)
}
}

void IntfsOrch::removeIp2MeRoute(const IpPrefix &ip_prefix)
void IntfsOrch::removeIp2MeRoute(string &vrf_name, const IpPrefix &ip_prefix)
{
sai_route_entry_t unicast_route_entry;
unicast_route_entry.switch_id = gSwitchId;
unicast_route_entry.vr_id = gVirtualRouterId;
unicast_route_entry.vr_id = m_vrfOrch->getVRFid(vrf_name);
copy(unicast_route_entry.destination, ip_prefix.getIp());

sai_status_t status = sai_route_api->remove_route_entry(&unicast_route_entry);
Expand Down
10 changes: 6 additions & 4 deletions orchagent/intfsorch.h
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

#include "orch.h"
#include "portsorch.h"
#include "vrforch.h"

#include "ipaddresses.h"
#include "ipprefix.h"
Expand All @@ -25,7 +26,7 @@ typedef map<string, IntfsEntry> IntfsTable;
class IntfsOrch : public Orch
{
public:
IntfsOrch(DBConnector *db, string tableName);
IntfsOrch(DBConnector *db, string tableName, VRFOrch *vrf_orch);

sai_object_id_t getRouterIntfsId(const string&);

Expand All @@ -35,19 +36,20 @@ class IntfsOrch : public Orch
bool setRouterIntfsMtu(Port &port);
std::set<IpPrefix> getSubnetRoutes();
private:
VRFOrch *m_vrfOrch;
IntfsTable m_syncdIntfses;
void doTask(Consumer &consumer);

int getRouterIntfsRefCount(const string&);

bool addRouterIntfs(Port &port);
bool addRouterIntfs(string &vrf_name, Port &port);
bool removeRouterIntfs(Port &port);

void addSubnetRoute(const Port &port, const IpPrefix &ip_prefix);
void removeSubnetRoute(const Port &port, const IpPrefix &ip_prefix);

void addIp2MeRoute(const IpPrefix &ip_prefix);
void removeIp2MeRoute(const IpPrefix &ip_prefix);
void addIp2MeRoute(string &vrf_name, const IpPrefix &ip_prefix);
void removeIp2MeRoute(string &vrf_name, const IpPrefix &ip_prefix);

void addDirectedBroadcast(const Port &port, const IpAddress &ip_addr);
void removeDirectedBroadcast(const Port &port, const IpAddress &ip_addr);
Expand Down
10 changes: 7 additions & 3 deletions orchagent/orchdaemon.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -69,12 +69,16 @@ bool OrchDaemon::init()

gCrmOrch = new CrmOrch(m_configDb, CFG_CRM_TABLE_NAME);
gPortsOrch = new PortsOrch(m_applDb, ports_tables);

TableConnector applDbFdb(m_applDb, APP_FDB_TABLE_NAME);
TableConnector stateDbFdb(m_stateDb, STATE_FDB_TABLE_NAME);
gFdbOrch = new FdbOrch(applDbFdb, stateDbFdb, gPortsOrch);

gIntfsOrch = new IntfsOrch(m_applDb, APP_INTF_TABLE_NAME);
VNetOrch *vnet_orch = new VNetOrch(m_applDb, APP_VNET_TABLE_NAME);
gDirectory.set(vnet_orch);
VRFOrch *vrf_orch = new VRFOrch(m_applDb, APP_VRF_TABLE_NAME);
gDirectory.set(vrf_orch);

gIntfsOrch = new IntfsOrch(m_applDb, APP_INTF_TABLE_NAME, vrf_orch);
gNeighOrch = new NeighOrch(m_applDb, APP_NEIGH_TABLE_NAME, gIntfsOrch);
gRouteOrch = new RouteOrch(m_applDb, APP_ROUTE_TABLE_NAME, gNeighOrch);
CoppOrch *copp_orch = new CoppOrch(m_applDb, APP_COPP_TABLE_NAME);
Expand Down Expand Up @@ -111,7 +115,6 @@ bool OrchDaemon::init()
TableConnector stateDbMirrorSession(m_stateDb, APP_MIRROR_SESSION_TABLE_NAME);
TableConnector confDbMirrorSession(m_configDb, CFG_MIRROR_SESSION_TABLE_NAME);
MirrorOrch *mirror_orch = new MirrorOrch(stateDbMirrorSession, confDbMirrorSession, gPortsOrch, gRouteOrch, gNeighOrch, gFdbOrch);
VRFOrch *vrf_orch = new VRFOrch(m_configDb, CFG_VRF_TABLE_NAME);

TableConnector confDbAclTable(m_configDb, CFG_ACL_TABLE_NAME);
TableConnector confDbAclRuleTable(m_configDb, CFG_ACL_RULE_TABLE_NAME);
Expand Down Expand Up @@ -178,6 +181,7 @@ bool OrchDaemon::init()
m_orchList.push_back(gFdbOrch);
m_orchList.push_back(mirror_orch);
m_orchList.push_back(gAclOrch);
m_orchList.push_back(vnet_orch);
m_orchList.push_back(vrf_orch);
m_orchList.push_back(vxlan_tunnel_orch);
m_orchList.push_back(vxlan_tunnel_map_orch);
Expand Down
1 change: 1 addition & 0 deletions orchagent/orchdaemon.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#include "crmorch.h"
#include "vrforch.h"
#include "vxlanorch.h"
#include "vnetorch.h"
#include "countercheckorch.h"
#include "flexcounterorch.h"
#include "directory.h"
Expand Down
1 change: 1 addition & 0 deletions orchagent/port.h
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ class Port
sai_object_id_t m_bridge_port_id = 0; // TODO: port could have multiple bridge port IDs
sai_vlan_id_t m_port_vlan_id = DEFAULT_PORT_VLAN_ID; // Port VLAN ID
sai_object_id_t m_rif_id = 0;
sai_object_id_t m_vr_id = 0;
sai_object_id_t m_hif_id = 0;
sai_object_id_t m_lag_id = 0;
sai_object_id_t m_lag_member_id = 0;
Expand Down
23 changes: 22 additions & 1 deletion orchagent/request_parser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
#include <vector>
#include <unordered_map>
#include <unordered_set>
#include <set>
#include <exception>

#include "sai.h"
Expand Down Expand Up @@ -145,6 +144,9 @@ void Request::parseAttrs(const KeyOpFieldsValuesTuple& request)
case REQ_T_UINT:
attr_item_uint_[fvField(*i)] = parseUint(fvValue(*i));
break;
case REQ_T_SET:
attr_item_set_[fvField(*i)] = parseSet(fvValue(*i));
break;
default:
throw std::logic_error(std::string("Not implemented attribute type parser for attribute:") + fvField(*i));
}
Expand Down Expand Up @@ -207,6 +209,25 @@ IpAddress Request::parseIpAddress(const std::string& str)
}
}

set<string> Request::parseSet(const std::string& str)
{
try
{
set<string> str_set;
string substr;
std::istringstream iss(str);
while (getline(iss, substr, ','))
{
str_set.insert(str);
}
return str_set;
}
catch (std::invalid_argument& _)
{
throw std::invalid_argument(std::string("Invalid string set"));
}
}

uint64_t Request::parseUint(const std::string& str)
{
try
Expand Down
11 changes: 11 additions & 0 deletions orchagent/request_parser.h
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
#define __REQUEST_PARSER_H

#include "ipaddress.h"
#include <sstream>
#include <set>

typedef enum _request_types_t
{
Expand All @@ -13,6 +15,7 @@ typedef enum _request_types_t
REQ_T_IP,
REQ_T_VLAN,
REQ_T_UINT,
REQ_T_SET,
} request_types_t;

typedef struct _request_description
Expand Down Expand Up @@ -112,6 +115,12 @@ class Request
return attr_item_uint_.at(attr_name);
}

const set<string>& getAttrSet(const std::string& attr_name) const
{
assert(is_parsed_);
return attr_item_set_.at(attr_name);
}

protected:
Request(const request_description_t& request_description, const char key_separator)
: request_description_(request_description),
Expand All @@ -131,6 +140,7 @@ class Request
IpAddress parseIpAddress(const std::string& str);
uint64_t parseUint(const std::string& str);
uint16_t parseVlan(const std::string& str);
set<string> parseSet(const std::string& str);

sai_packet_action_t parsePacketAction(const std::string& str);

Expand All @@ -154,6 +164,7 @@ class Request
std::unordered_map<std::string, uint16_t> attr_item_vlan_;
std::unordered_map<std::string, IpAddress> attr_item_ip_;
std::unordered_map<std::string, uint64_t> attr_item_uint_;
std::unordered_map<std::string, set<string>> attr_item_set_;
};

#endif // __REQUEST_PARSER_H
Loading