Skip to content

Commit

Permalink
Resolve code review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@qiluo-msft
qiluo-msft committed Apr 14, 2017
1 parent 3ef44b5 commit cddc4e1
Show file tree
Hide file tree
Showing 5 changed files with 74 additions and 47 deletions.
9 changes: 0 additions & 9 deletions orchagent/aclorch.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1170,13 +1170,10 @@ sai_status_t AclOrch::createBindAclTable(AclTable &aclTable, sai_object_id_t &ta
sai_status_t status;
sai_attribute_t attr;
vector<sai_attribute_t> table_attrs;
// workaround until SAI is fixed
#if 0
int32_t range_types_list[] =
{ SAI_ACL_RANGE_TYPE_L4_DST_PORT_RANGE,
SAI_ACL_RANGE_TYPE_L4_SRC_PORT_RANGE
};
#endif

attr.id = SAI_ACL_TABLE_ATTR_ACL_BIND_POINT_TYPE_LIST;
attr.value.s32 = SAI_ACL_BIND_POINT_TYPE_PORT;
Expand Down Expand Up @@ -1230,15 +1227,9 @@ sai_status_t AclOrch::createBindAclTable(AclTable &aclTable, sai_object_id_t &ta
}

attr.id = SAI_ACL_TABLE_ATTR_FIELD_ACL_RANGE_TYPE;
// workaround until SAI is fixed
#if 0
attr.value.s32list.count = sizeof(range_types_list) / sizeof(range_types_list[0]);
attr.value.s32list.list = range_types_list;
table_attrs.push_back(attr);
#else
attr.value.s32list.count = 0;
table_attrs.push_back(attr);
#endif

status = sai_acl_api->create_acl_table(&table_oid, gSwitchId, table_attrs.size(), table_attrs.data());

Expand Down
54 changes: 38 additions & 16 deletions orchagent/copporch.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,12 +106,6 @@ void CoppOrch::initDefaultTrapIds()
attr.value.oid = m_trap_group_map[default_trap_group];
trap_id_attrs.push_back(attr);

/* Receive packets via OS net device */
// TODO: double check semantics
attr.id = SAI_HOSTIF_ATTR_TYPE;
attr.value.s32 = SAI_HOSTIF_TYPE_NETDEV;
trap_id_attrs.push_back(attr);

if (!applyAttributesToTrapIds(m_trap_group_map[default_trap_group], default_trap_ids, trap_id_attrs))
{
SWSS_LOG_ERROR("Failed to set attributes to default trap IDs");
Expand Down Expand Up @@ -154,17 +148,52 @@ bool CoppOrch::applyAttributesToTrapIds(sai_object_id_t trap_group_id,
const vector<sai_hostif_trap_type_t> &trap_id_list,
vector<sai_attribute_t> &trap_id_attribs)
{
sai_status_t status;

vector<sai_attribute_t> attrs(2);
attrs[0].id = SAI_HOSTIF_TRAP_ATTR_TRAP_GROUP;
attrs[0].value.oid = trap_group_id;
attrs[1].id = SAI_HOSTIF_TRAP_ATTR_TRAP_TYPE;

for (auto trap_id : trap_id_list)
{
for (auto attr : trap_id_attribs)
auto found = m_trap_type_map.find(trap_id);
if (found == m_trap_type_map.end())
{
attrs.resize(2);
attrs[1].value.s32 = trap_id;
attrs.insert(attrs.end(), trap_id_attribs.begin(), trap_id_attribs.end());

sai_object_id_t hostif_trap_id;
status = sai_hostif_api->create_hostif_trap(&hostif_trap_id, gSwitchId, attrs.size(), attrs.data());
if (status != SAI_STATUS_SUCCESS)
{
SWSS_LOG_ERROR("Failed to create trap %d, rc=%d", trap_id, status);
return false;
}
m_syncdTrapIds[trap_id] = hostif_trap_id;
}
else
{
sai_status_t status = sai_hostif_api->set_hostif_trap_attribute(trap_id, &attr);
// Set caller provided attributes
for (auto attr : trap_id_attribs)
{
status = sai_hostif_api->set_hostif_trap_attribute(trap_id, &attr);
if (status != SAI_STATUS_SUCCESS)
{
SWSS_LOG_ERROR("Failed to set attribute %d to trap %d, rc=%d", attr.id, trap_id, status);
return false;
}
}
// Set the trap group attribute
status = sai_hostif_api->set_hostif_trap_attribute(trap_id, &attrs[1]);
if (status != SAI_STATUS_SUCCESS)
{
SWSS_LOG_ERROR("Failed to set attribute %d to trap %d, rc=%d", attr.id, trap_id, status);
SWSS_LOG_ERROR("Failed to set trap group attribute to trap %d, rc=%d", trap_id, status);
return false;
}
}

m_syncdTrapIds[trap_id] = trap_group_id;
}

Expand All @@ -183,12 +212,6 @@ bool CoppOrch::applyTrapIds(sai_object_id_t trap_group, vector<string> &trap_id_
attr.value.oid = trap_group;
trap_id_attribs.push_back(attr);

/* Receive packets via OS net device */
// TODO: double check semantics
attr.id = SAI_HOSTIF_ATTR_TYPE;
attr.value.s32 = SAI_HOSTIF_TYPE_NETDEV;
trap_id_attribs.push_back(attr);

return applyAttributesToTrapIds(trap_group, trap_id_list, trap_id_attribs);
}

Expand Down Expand Up @@ -597,4 +620,3 @@ void CoppOrch::doTask(Consumer &consumer)
}
}
}

3 changes: 2 additions & 1 deletion orchagent/copporch.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,15 @@ const string copp_policer_action_yellow_field = "yellow_action";
/* TrapGroupPolicerTable: trap group ID, policer ID */
typedef map<sai_object_id_t, sai_object_id_t> TrapGroupPolicerTable;
/* TrapIdTrapGroupTable: trap ID, trap group ID */
typedef map<sai_hostif_trap_type_t, sai_object_id_t> TrapIdTrapGroupTable;
typedef map<sai_hostif_trap_type_t, sai_object_id_t> TrapIdTrapGroupTable, TrapTypeMap;

class CoppOrch : public Orch
{
public:
CoppOrch(DBConnector *db, string tableName);
protected:
object_map m_trap_group_map;
TrapTypeMap m_trap_type_map;

TrapGroupPolicerTable m_trap_group_policer_map;
TrapIdTrapGroupTable m_syncdTrapIds;
Expand Down
13 changes: 9 additions & 4 deletions orchagent/main.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ void on_fdb_event(uint32_t count, sai_fdb_event_notification_data_t *data);

/* Initialize all global api pointers */
sai_switch_api_t* sai_switch_api;
sai_bridge_api_t* sai_bridge_api;
sai_virtual_router_api_t* sai_virtual_router_api;
sai_port_api_t* sai_port_api;
sai_vlan_api_t* sai_vlan_api;
Expand Down Expand Up @@ -91,6 +92,7 @@ void initSaiApi()
sai_api_initialize(0, (service_method_table_t *)&test_services);

sai_api_query(SAI_API_SWITCH, (void **)&sai_switch_api);
sai_api_query(SAI_API_BRIDGE, (void **)&sai_bridge_api);
sai_api_query(SAI_API_VIRTUAL_ROUTER, (void **)&sai_virtual_router_api);
sai_api_query(SAI_API_PORT, (void **)&sai_port_api);
sai_api_query(SAI_API_FDB, (void **)&sai_fdb_api);
Expand All @@ -114,6 +116,7 @@ void initSaiApi()
sai_api_query(SAI_API_ACL, (void **)&sai_acl_api);

sai_log_set(SAI_API_SWITCH, SAI_LOG_LEVEL_NOTICE);
sai_log_set(SAI_API_BRIDGE, SAI_LOG_LEVEL_NOTICE);
sai_log_set(SAI_API_VIRTUAL_ROUTER, SAI_LOG_LEVEL_NOTICE);
sai_log_set(SAI_API_PORT, SAI_LOG_LEVEL_NOTICE);
sai_log_set(SAI_API_FDB, SAI_LOG_LEVEL_NOTICE);
Expand Down Expand Up @@ -171,10 +174,12 @@ int main(int argc, char **argv)

SWSS_LOG_NOTICE("sai_switch_api: create a switch");

sai_attribute_t switch_attr;
switch_attr.id = SAI_SWITCH_ATTR_FDB_EVENT_NOTIFY;
switch_attr.value.ptr = (void *)on_fdb_event;
status = sai_switch_api->create_switch(&gSwitchId, 1, &switch_attr);
sai_attribute_t switch_attrs[2];
switch_attrs[0].id = SAI_SWITCH_ATTR_FDB_EVENT_NOTIFY;
switch_attrs[0].value.ptr = (void *)on_fdb_event;
switch_attrs[1].id = SAI_SWITCH_ATTR_INIT_SWITCH;
switch_attrs[1].value.booldata = true;
status = sai_switch_api->create_switch(&gSwitchId, 2, switch_attrs);
if (status != SAI_STATUS_SUCCESS)
{
SWSS_LOG_ERROR("Failed to create a switch %d", status);
Expand Down
42 changes: 25 additions & 17 deletions orchagent/portsorch.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include "schema.h"

extern sai_switch_api_t *sai_switch_api;
extern sai_bridge_api_t *sai_bridge_api;
extern sai_port_api_t *sai_port_api;
extern sai_vlan_api_t *sai_vlan_api;
extern sai_lag_api_t *sai_lag_api;
Expand Down Expand Up @@ -110,9 +111,8 @@ PortsOrch::PortsOrch(DBConnector *db, vector<string> tableNames) :
/* Set port to hardware learn mode */
for (i = 0; i < m_portCount; i++)
{
// TODO: find ??
// attr.id = SAI_PORT_ATTR_FDB_LEARNING;
// attr.value.s32 = SAI_PORT_LEARN_MODE_HW;
attr.id = SAI_BRIDGE_PORT_ATTR_FDB_LEARNING_MODE;
attr.value.s32 = SAI_BRIDGE_PORT_FDB_LEARNING_MODE_HW;

status = sai_port_api->set_port_attribute(port_list[i], &attr);
if (status != SAI_STATUS_SUCCESS)
Expand All @@ -123,28 +123,36 @@ PortsOrch::PortsOrch(DBConnector *db, vector<string> tableNames) :
SWSS_LOG_NOTICE("Set port to hardware learn mode pid:%lx", port_list[i]);
}

/* Get default VLAN member list */
vector<sai_object_id_t> vlan_member_list;
vlan_member_list.resize(m_portCount);

attr.id = SAI_VLAN_ATTR_MEMBER_LIST;
attr.value.objlist.count = vlan_member_list.size();
attr.value.objlist.list = vlan_member_list.data();
// Get default bridge id
sai_attribute_t switch_attr;
switch_attr.id = SAI_SWITCH_ATTR_DEFAULT_1Q_BRIDGE_ID;
status = sai_switch_api->get_switch_attribute(gSwitchId, 1, &switch_attr);
if (status != SAI_STATUS_SUCCESS)
{
SWSS_LOG_ERROR("Failed to get default 1Q_BRIDGE: %d", status);
throw "PortsOrch initialization failure";
}
sai_object_id_t bridge_id = switch_attr.value.oid;

status = sai_vlan_api->get_vlan_attribute(DEFAULT_VLAN_ID, 1, &attr);
// Get bridge port list for default bridge id
sai_attribute_t bridge_attr;
bridge_attr.id = SAI_BRIDGE_ATTR_PORT_LIST;
bridge_attr.value.objlist.count = 100;
status = sai_bridge_api->get_bridge_attribute(bridge_id, 1, &bridge_attr);
if (status != SAI_STATUS_SUCCESS)
{
SWSS_LOG_ERROR("Failed to get default VLAN member list");
SWSS_LOG_ERROR("Failed to get default 1Q_BRIDGE: %d", status);
throw "PortsOrch initialization failure";
}

/* Remove port from default VLAN */
for (i = 0; i < attr.value.objlist.count; i++)
// Remove every bridge port from default bridge
for (i = 0; i < bridge_attr.value.objlist.count; ++i)
{
status = sai_vlan_api->remove_vlan_member(vlan_member_list[i]);
sai_object_id_t bridge_port_id = bridge_attr.value.objlist.list[i];
status = sai_bridge_api->remove_bridge_port(bridge_port_id);
if (status != SAI_STATUS_SUCCESS)
{
SWSS_LOG_ERROR("Failed to remove port from default VLAN %d", i);
SWSS_LOG_ERROR("Failed to remove port %lx from default 1Q_BRIDGE: %d", bridge_port_id, status);
throw "PortsOrch initialization failure";
}
}
Expand Down Expand Up @@ -817,7 +825,7 @@ bool PortsOrch::addHostIntfs(sai_object_id_t id, string alias, sai_object_id_t &
bool PortsOrch::addVlan(string vlan_alias)
{
SWSS_LOG_ENTER();

// TODO: how to use vlan_oid??
sai_object_id_t vlan_oid;

Expand Down

0 comments on commit cddc4e1

Please sign in to comment.