From 5bc31f27a7a5133a611ed26ad2eabdc3e1349d75 Mon Sep 17 00:00:00 2001 From: dgsudharsan Date: Fri, 26 Jul 2019 11:50:27 -0700 Subject: [PATCH 1/7] Copp sflow changes undoing revert indentation Code review fixes Addressing code review comments Latest copp changes based on headers --- orchagent/copporch.cpp | 164 ++++++++++++++++++++++++++ orchagent/copporch.h | 14 +++ swssconfig/sample/00-copp.config.json | 16 +++ 3 files changed, 194 insertions(+) diff --git a/orchagent/copporch.cpp b/orchagent/copporch.cpp index 6e3c711d42..cce513ba31 100644 --- a/orchagent/copporch.cpp +++ b/orchagent/copporch.cpp @@ -212,6 +212,41 @@ bool CoppOrch::applyAttributesToTrapIds(sai_object_id_t trap_group_id, return false; } m_syncdTrapIds[trap_id] = trap_group_id; + + auto hostTbl_entry = m_trapId_hostTblEntry_map.find(trap_id); + + if (hostTbl_entry == m_trapId_hostTblEntry_map.end()) + { + auto hostif_map = m_trap_group_hostif_map.find(trap_group_id); + if (hostif_map != m_trap_group_hostif_map.end()) + { + sai_object_id_t hostif_table_entry = SAI_NULL_OBJECT_ID; + sai_attribute_t sai_host_table_attr[4]; + + sai_host_table_attr[0].id = SAI_HOSTIF_TABLE_ENTRY_ATTR_TYPE; + sai_host_table_attr[0].value.s32 = SAI_HOSTIF_TABLE_ENTRY_TYPE_TRAP_ID; + + sai_host_table_attr[1].id = SAI_HOSTIF_TABLE_ENTRY_ATTR_TRAP_ID; + sai_host_table_attr[1].value.oid = hostif_trap_id; + + sai_host_table_attr[2].id = SAI_HOSTIF_TABLE_ENTRY_ATTR_CHANNEL_TYPE; + sai_host_table_attr[2].value.s32 = SAI_HOSTIF_TABLE_ENTRY_CHANNEL_TYPE_GENETLINK; + + sai_host_table_attr[3].id = SAI_HOSTIF_TABLE_ENTRY_ATTR_HOST_IF; + sai_host_table_attr[3].value.oid = hostif_map->second; + + sai_status_t status = sai_hostif_api->create_hostif_table_entry(&hostif_table_entry, + gSwitchId, 4, + sai_host_table_attr); + + if (status != SAI_STATUS_SUCCESS) + { + SWSS_LOG_ERROR("Failed to create hostif table entry failed, rv %d", status); + return false; + } + m_trapId_hostTblEntry_map[trap_id] = hostif_table_entry; + } + } } return true; @@ -319,6 +354,69 @@ bool CoppOrch::createPolicer(string trap_group_name, vector &po return true; } +bool CoppOrch::createGenetlinkHostIf(string trap_group_name, vector &genetlink_attribs) +{ + SWSS_LOG_ENTER(); + + sai_object_id_t hostif_id; + sai_status_t sai_status; + + sai_status = sai_hostif_api->create_hostif(&hostif_id, gSwitchId, + (uint32_t)genetlink_attribs.size(), + genetlink_attribs.data()); + if (sai_status != SAI_STATUS_SUCCESS) + { + SWSS_LOG_ERROR("Failed to create genetlink hostif for trap group %s, rc=%d", + trap_group_name.c_str(), sai_status); + return false; + } + + m_trap_group_hostif_map[m_trap_group_map[trap_group_name]] = hostif_id; + return true; +} + +bool CoppOrch::removeGenetlinkHostIf(string trap_group_name) +{ + SWSS_LOG_ENTER(); + + sai_status_t sai_status; + + for (auto it : m_syncdTrapIds) + { + if (it.second == m_trap_group_map[trap_group_name]) + { + auto hostTableEntry = m_trapId_hostTblEntry_map.find(it.first); + if (hostTableEntry != m_trapId_hostTblEntry_map.end()) + { + sai_status = sai_hostif_api->remove_hostif_table_entry(hostTableEntry->second); + if(sai_status != SAI_STATUS_SUCCESS) + { + SWSS_LOG_ERROR("Failed to delete hostif table entry %ld \ + on trap group %s. rc=%d", hostTableEntry->second, + trap_group_name.c_str(), sai_status); + return false; + } + m_trapId_hostTblEntry_map.erase(it.first); + } + } + } + + auto hostInfo = m_trap_group_hostif_map.find(m_trap_group_map[trap_group_name]); + if(hostInfo != m_trap_group_hostif_map.end()) + { + sai_status = sai_hostif_api->remove_hostif(hostInfo->second); + if(sai_status != SAI_STATUS_SUCCESS) + { + SWSS_LOG_ERROR("Failed to delete host info %ld on trap group %s. rc=%d", + hostInfo->second, trap_group_name.c_str(), sai_status); + return false; + } + m_trap_group_hostif_map.erase(m_trap_group_map[trap_group_name]); + } + + return true; +} + task_process_status CoppOrch::processCoppRule(Consumer& consumer) { SWSS_LOG_ENTER(); @@ -334,6 +432,7 @@ task_process_status CoppOrch::processCoppRule(Consumer& consumer) vector trap_gr_attribs; vector trap_id_attribs; vector policer_attribs; + vector genetlink_attribs; if (op == SET_COMMAND) { @@ -441,6 +540,25 @@ task_process_status CoppOrch::processCoppRule(Consumer& consumer) attr.value.s32 = policer_action; policer_attribs.push_back(attr); } + else if (fvField(*i) == copp_genetlink_name) + { + attr.id = SAI_HOSTIF_ATTR_TYPE; + attr.value.s32 = SAI_HOSTIF_TYPE_GENETLINK; + genetlink_attribs.push_back(attr); + + attr.id = SAI_HOSTIF_ATTR_NAME; + strncpy(attr.value.chardata, fvValue(*i).c_str(), + sizeof(attr.value.chardata)); + genetlink_attribs.push_back(attr); + + } + else if (fvField(*i) == copp_genetlink_mcgrp_name) + { + attr.id = SAI_HOSTIF_ATTR_GENETLINK_MCGRP_NAME; + strncpy(attr.value.chardata, fvValue(*i).c_str(), + sizeof(attr.value.chardata)); + genetlink_attribs.push_back(attr); + } else { SWSS_LOG_ERROR("Unknown copp field specified:%s\n", fvField(*i).c_str()); @@ -480,6 +598,38 @@ task_process_status CoppOrch::processCoppRule(Consumer& consumer) } } } + if (!genetlink_attribs.empty()) + { + auto hostif_map = m_trap_group_hostif_map.find(m_trap_group_map[trap_group_name]); + + if (hostif_map != m_trap_group_hostif_map.end()) + { + for(sai_uint32_t idx = 0; idx < genetlink_attribs.size(); idx++) + { + auto hostif_attr = genetlink_attribs[idx]; + sai_status = sai_hostif_api->set_hostif_attribute(hostif_map->second, + &hostif_attr); + if(sai_status != SAI_STATUS_SUCCESS) + { + SWSS_LOG_ERROR("Failed to apply attribute[%d].id=%d to hostif for \ + trap group %s, error:%d", idx, hostif_attr.id, + trap_group_name.c_str(), sai_status); + + return task_process_status::task_failed; + } + } + } + else + { + if (!genetlink_attribs.empty()) + { + if (!createGenetlinkHostIf(trap_group_name, genetlink_attribs)) + { + return task_process_status::task_failed; + } + } + } + } for (sai_uint32_t ind = 0; ind < trap_gr_attribs.size(); ind++) { @@ -517,6 +667,14 @@ task_process_status CoppOrch::processCoppRule(Consumer& consumer) return task_process_status::task_failed; } } + + if (!genetlink_attribs.empty()) + { + if (!createGenetlinkHostIf(trap_group_name, genetlink_attribs)) + { + return task_process_status::task_failed; + } + } } /* Apply traps to trap group */ @@ -534,6 +692,12 @@ task_process_status CoppOrch::processCoppRule(Consumer& consumer) return task_process_status::task_failed; } + if (!removeGenetlinkHostIf(trap_group_name)) + { + SWSS_LOG_ERROR("Failed to remove hostif from trap group %s", trap_group_name.c_str()); + return task_process_status::task_failed; + } + /* Do not remove default trap group */ if (trap_group_name == default_trap_group) { diff --git a/orchagent/copporch.h b/orchagent/copporch.h index 5fc5bc2ae8..faffad4493 100644 --- a/orchagent/copporch.h +++ b/orchagent/copporch.h @@ -24,10 +24,18 @@ const std::string copp_policer_action_green_field = "green_action"; const std::string copp_policer_action_red_field = "red_action"; const std::string copp_policer_action_yellow_field = "yellow_action"; +// genetlink fields +const string copp_genetlink_name = "genetlink_name"; +const string copp_genetlink_mcgrp_name = "genetlink_mcgrp_name"; + /* TrapGroupPolicerTable: trap group ID, policer ID */ typedef std::map TrapGroupPolicerTable; /* TrapIdTrapGroupTable: trap ID, trap group ID */ typedef std::map TrapIdTrapGroupTable; +/* TrapGroupHostIfMap: trap group ID to host interface ID */ +typedef std::map TrapGroupHostIfMap; +/* TrapGroupHostIfMap: trap type to host table entry */ +typedef std::map TrapHostTblEntryMap; class CoppOrch : public Orch { @@ -39,6 +47,9 @@ class CoppOrch : public Orch TrapGroupPolicerTable m_trap_group_policer_map; TrapIdTrapGroupTable m_syncdTrapIds; + TrapGroupHostIfMap m_trap_group_hostif_map; + TrapHostTblEntryMap m_trapId_hostTblEntry_map; + void initDefaultHostIntfTable(); void initDefaultTrapGroup(); void initDefaultTrapIds(); @@ -54,6 +65,9 @@ class CoppOrch : public Orch sai_object_id_t getPolicer(std::string trap_group_name); + bool createGenetlinkHostIf(string trap_group_name, vector &hostif_attribs); + bool removeGenetlinkHostIf(string trap_group_name); + virtual void doTask(Consumer& consumer); }; #endif /* SWSS_COPPORCH_H */ diff --git a/swssconfig/sample/00-copp.config.json b/swssconfig/sample/00-copp.config.json index e82242830c..a4742d79b1 100644 --- a/swssconfig/sample/00-copp.config.json +++ b/swssconfig/sample/00-copp.config.json @@ -55,5 +55,21 @@ "red_action":"drop" }, "OP": "SET" + }, + { + "COPP_TABLE:trap.group.sflow": { + "trap_ids": "sample_packet", + "trap_action":"trap", + "trap_priority":"1", + "queue": "2", + "meter_type":"packets", + "mode":"sr_tcm", + "cir":"1000", + "cbs":"1000", + "red_action":"drop", + "genetlink_name":"psample", + "genetlink_mcgrp_name":"packets" + }, + "OP": "SET" } ] From 761678b982cbba7e1450b5dec3528912a60c0854 Mon Sep 17 00:00:00 2001 From: dgsudharsan Date: Mon, 16 Sep 2019 09:57:41 -0700 Subject: [PATCH 2/7] Addressing code review comments Addressing code review comments --- orchagent/copporch.cpp | 30 ++++++++++++++++----------- swssconfig/sample/00-copp.config.json | 16 -------------- 2 files changed, 18 insertions(+), 28 deletions(-) diff --git a/orchagent/copporch.cpp b/orchagent/copporch.cpp index cce513ba31..381ace87a8 100644 --- a/orchagent/copporch.cpp +++ b/orchagent/copporch.cpp @@ -221,23 +221,29 @@ bool CoppOrch::applyAttributesToTrapIds(sai_object_id_t trap_group_id, if (hostif_map != m_trap_group_hostif_map.end()) { sai_object_id_t hostif_table_entry = SAI_NULL_OBJECT_ID; - sai_attribute_t sai_host_table_attr[4]; + sai_attribute_t attr; + vector sai_host_table_attr; - sai_host_table_attr[0].id = SAI_HOSTIF_TABLE_ENTRY_ATTR_TYPE; - sai_host_table_attr[0].value.s32 = SAI_HOSTIF_TABLE_ENTRY_TYPE_TRAP_ID; + attr.id = SAI_HOSTIF_TABLE_ENTRY_ATTR_TYPE; + attr.value.s32 = SAI_HOSTIF_TABLE_ENTRY_TYPE_TRAP_ID; + sai_host_table_attr.push_back(attr); - sai_host_table_attr[1].id = SAI_HOSTIF_TABLE_ENTRY_ATTR_TRAP_ID; - sai_host_table_attr[1].value.oid = hostif_trap_id; + attr.id = SAI_HOSTIF_TABLE_ENTRY_ATTR_TRAP_ID; + attr.value.oid = hostif_trap_id; + sai_host_table_attr.push_back(attr); - sai_host_table_attr[2].id = SAI_HOSTIF_TABLE_ENTRY_ATTR_CHANNEL_TYPE; - sai_host_table_attr[2].value.s32 = SAI_HOSTIF_TABLE_ENTRY_CHANNEL_TYPE_GENETLINK; + attr.id = SAI_HOSTIF_TABLE_ENTRY_ATTR_CHANNEL_TYPE; + attr.value.s32 = SAI_HOSTIF_TABLE_ENTRY_CHANNEL_TYPE_GENETLINK; + sai_host_table_attr.push_back(attr); - sai_host_table_attr[3].id = SAI_HOSTIF_TABLE_ENTRY_ATTR_HOST_IF; - sai_host_table_attr[3].value.oid = hostif_map->second; + attr.id = SAI_HOSTIF_TABLE_ENTRY_ATTR_HOST_IF; + attr.value.oid = hostif_map->second; + sai_host_table_attr.push_back(attr); sai_status_t status = sai_hostif_api->create_hostif_table_entry(&hostif_table_entry, - gSwitchId, 4, - sai_host_table_attr); + gSwitchId, + (uint32_t)sai_host_table_attr.size(), + sai_host_table_attr.data()); if (status != SAI_STATUS_SUCCESS) { @@ -615,7 +621,7 @@ task_process_status CoppOrch::processCoppRule(Consumer& consumer) trap group %s, error:%d", idx, hostif_attr.id, trap_group_name.c_str(), sai_status); - return task_process_status::task_failed; + return task_process_status::task_ignore; } } } diff --git a/swssconfig/sample/00-copp.config.json b/swssconfig/sample/00-copp.config.json index a4742d79b1..e82242830c 100644 --- a/swssconfig/sample/00-copp.config.json +++ b/swssconfig/sample/00-copp.config.json @@ -55,21 +55,5 @@ "red_action":"drop" }, "OP": "SET" - }, - { - "COPP_TABLE:trap.group.sflow": { - "trap_ids": "sample_packet", - "trap_action":"trap", - "trap_priority":"1", - "queue": "2", - "meter_type":"packets", - "mode":"sr_tcm", - "cir":"1000", - "cbs":"1000", - "red_action":"drop", - "genetlink_name":"psample", - "genetlink_mcgrp_name":"packets" - }, - "OP": "SET" } ] From 5d0960cfc62f4e136b3628c8620d9534cdb0e9c1 Mon Sep 17 00:00:00 2001 From: dgsudharsan Date: Wed, 25 Sep 2019 16:54:16 -0700 Subject: [PATCH 3/7] Addressing review comments --- orchagent/copporch.cpp | 12 ++++++------ orchagent/copporch.h | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/orchagent/copporch.cpp b/orchagent/copporch.cpp index 381ace87a8..664844a9b2 100644 --- a/orchagent/copporch.cpp +++ b/orchagent/copporch.cpp @@ -213,9 +213,9 @@ bool CoppOrch::applyAttributesToTrapIds(sai_object_id_t trap_group_id, } m_syncdTrapIds[trap_id] = trap_group_id; - auto hostTbl_entry = m_trapId_hostTblEntry_map.find(trap_id); + auto hostTbl_entry = m_trapid_hostif_table_map.find(trap_id); - if (hostTbl_entry == m_trapId_hostTblEntry_map.end()) + if (hostTbl_entry == m_trapid_hostif_table_map.end()) { auto hostif_map = m_trap_group_hostif_map.find(trap_group_id); if (hostif_map != m_trap_group_hostif_map.end()) @@ -250,7 +250,7 @@ bool CoppOrch::applyAttributesToTrapIds(sai_object_id_t trap_group_id, SWSS_LOG_ERROR("Failed to create hostif table entry failed, rv %d", status); return false; } - m_trapId_hostTblEntry_map[trap_id] = hostif_table_entry; + m_trapid_hostif_table_map[trap_id] = hostif_table_entry; } } } @@ -391,8 +391,8 @@ bool CoppOrch::removeGenetlinkHostIf(string trap_group_name) { if (it.second == m_trap_group_map[trap_group_name]) { - auto hostTableEntry = m_trapId_hostTblEntry_map.find(it.first); - if (hostTableEntry != m_trapId_hostTblEntry_map.end()) + auto hostTableEntry = m_trapid_hostif_table_map.find(it.first); + if (hostTableEntry != m_trapid_hostif_table_map.end()) { sai_status = sai_hostif_api->remove_hostif_table_entry(hostTableEntry->second); if(sai_status != SAI_STATUS_SUCCESS) @@ -402,7 +402,7 @@ bool CoppOrch::removeGenetlinkHostIf(string trap_group_name) trap_group_name.c_str(), sai_status); return false; } - m_trapId_hostTblEntry_map.erase(it.first); + m_trapid_hostif_table_map.erase(it.first); } } } diff --git a/orchagent/copporch.h b/orchagent/copporch.h index faffad4493..71f247cdc7 100644 --- a/orchagent/copporch.h +++ b/orchagent/copporch.h @@ -48,7 +48,7 @@ class CoppOrch : public Orch TrapIdTrapGroupTable m_syncdTrapIds; TrapGroupHostIfMap m_trap_group_hostif_map; - TrapHostTblEntryMap m_trapId_hostTblEntry_map; + TrapIdHostIfTableMap m_trapid_hostif_table_map; void initDefaultHostIntfTable(); void initDefaultTrapGroup(); From 42a289fb0f0c7cc5ab07983234e377514356271c Mon Sep 17 00:00:00 2001 From: dgsudharsan Date: Fri, 4 Oct 2019 22:00:46 -0700 Subject: [PATCH 4/7] Handling sflow enable UT Fixes Fixing build Build fix --- orchagent/copporch.cpp | 124 ++++++++++++++++++++------ orchagent/copporch.h | 31 ++++--- orchagent/orchdaemon.cpp | 10 ++- swssconfig/sample/00-copp.config.json | 16 ++++ 4 files changed, 143 insertions(+), 38 deletions(-) diff --git a/orchagent/copporch.cpp b/orchagent/copporch.cpp index 664844a9b2..741c014e3c 100644 --- a/orchagent/copporch.cpp +++ b/orchagent/copporch.cpp @@ -7,6 +7,7 @@ #include #include #include +#include using namespace swss; using namespace std; @@ -88,14 +89,15 @@ const vector default_trap_ids = { SAI_HOSTIF_TRAP_TYPE_TTL_ERROR }; -CoppOrch::CoppOrch(DBConnector *db, string tableName) : - Orch(db, tableName) +CoppOrch::CoppOrch(vector &tableConnectors) : + Orch( tableConnectors) { SWSS_LOG_ENTER(); initDefaultHostIntfTable(); initDefaultTrapGroup(); initDefaultTrapIds(); + enable_sflow_trap = false; }; void CoppOrch::initDefaultHostIntfTable() @@ -189,31 +191,18 @@ void CoppOrch::getTrapIdList(vector &trap_id_name_list, vector &trap_id_list, - vector &trap_id_attribs) +bool CoppOrch::createGenetlinkHostifTable(vector &trap_id_name_list) { - for (auto trap_id : trap_id_list) - { - sai_attribute_t attr; - vector attrs; - - attr.id = SAI_HOSTIF_TRAP_ATTR_TRAP_TYPE; - attr.value.s32 = trap_id; - attrs.push_back(attr); + SWSS_LOG_ENTER(); - attrs.insert(attrs.end(), trap_id_attribs.begin(), trap_id_attribs.end()); + vector trap_id_list; - sai_object_id_t hostif_trap_id; - sai_status_t status = sai_hostif_api->create_hostif_trap(&hostif_trap_id, gSwitchId, (uint32_t)attrs.size(), attrs.data()); - if (status != SAI_STATUS_SUCCESS) - { - SWSS_LOG_ERROR("Failed to create trap %d, rv:%d", trap_id, status); - return false; - } - m_syncdTrapIds[trap_id] = trap_group_id; + getTrapIdList(trap_id_name_list, trap_id_list); + for (auto trap_id : trap_id_list) + { auto hostTbl_entry = m_trapid_hostif_table_map.find(trap_id); + sai_object_id_t trap_group_id = m_syncdTrapIds[trap_id].trap_group_obj; if (hostTbl_entry == m_trapid_hostif_table_map.end()) { @@ -229,7 +218,7 @@ bool CoppOrch::applyAttributesToTrapIds(sai_object_id_t trap_group_id, sai_host_table_attr.push_back(attr); attr.id = SAI_HOSTIF_TABLE_ENTRY_ATTR_TRAP_ID; - attr.value.oid = hostif_trap_id; + attr.value.oid = m_syncdTrapIds[trap_id].trap_obj; sai_host_table_attr.push_back(attr); attr.id = SAI_HOSTIF_TABLE_ENTRY_ATTR_CHANNEL_TYPE; @@ -244,7 +233,6 @@ bool CoppOrch::applyAttributesToTrapIds(sai_object_id_t trap_group_id, gSwitchId, (uint32_t)sai_host_table_attr.size(), sai_host_table_attr.data()); - if (status != SAI_STATUS_SUCCESS) { SWSS_LOG_ERROR("Failed to create hostif table entry failed, rv %d", status); @@ -254,7 +242,34 @@ bool CoppOrch::applyAttributesToTrapIds(sai_object_id_t trap_group_id, } } } + return true; +} +bool CoppOrch::applyAttributesToTrapIds(sai_object_id_t trap_group_id, + const vector &trap_id_list, + vector &trap_id_attribs) +{ + for (auto trap_id : trap_id_list) + { + sai_attribute_t attr; + vector attrs; + + attr.id = SAI_HOSTIF_TRAP_ATTR_TRAP_TYPE; + attr.value.s32 = trap_id; + attrs.push_back(attr); + + attrs.insert(attrs.end(), trap_id_attribs.begin(), trap_id_attribs.end()); + + sai_object_id_t hostif_trap_id; + sai_status_t status = sai_hostif_api->create_hostif_trap(&hostif_trap_id, gSwitchId, (uint32_t)attrs.size(), attrs.data()); + if (status != SAI_STATUS_SUCCESS) + { + SWSS_LOG_ERROR("Failed to create trap %d, rv:%d", trap_id, status); + return false; + } + m_syncdTrapIds[trap_id].trap_group_obj = trap_group_id; + m_syncdTrapIds[trap_id].trap_obj = hostif_trap_id; + } return true; } @@ -274,6 +289,7 @@ bool CoppOrch::applyTrapIds(sai_object_id_t trap_group, vector &trap_id_ return applyAttributesToTrapIds(trap_group, trap_id_list, trap_id_attribs); } + bool CoppOrch::removePolicer(string trap_group_name) { SWSS_LOG_ENTER(); @@ -389,7 +405,7 @@ bool CoppOrch::removeGenetlinkHostIf(string trap_group_name) for (auto it : m_syncdTrapIds) { - if (it.second == m_trap_group_map[trap_group_name]) + if (it.second.trap_group_obj == m_trap_group_map[trap_group_name]) { auto hostTableEntry = m_trapid_hostif_table_map.find(it.first); if (hostTableEntry != m_trapid_hostif_table_map.end()) @@ -449,6 +465,14 @@ task_process_status CoppOrch::processCoppRule(Consumer& consumer) if (fvField(*i) == copp_trap_id_list) { trap_id_list = tokenize(fvValue(*i), list_item_delimiter); + auto it = std::find(trap_id_list.begin(), trap_id_list.end(), "sample_packet"); + if (it != trap_id_list.end()) + { + if (!enable_sflow_trap) + { + return task_process_status::task_need_retry; + } + } } else if (fvField(*i) == copp_queue_field) { @@ -688,6 +712,14 @@ task_process_status CoppOrch::processCoppRule(Consumer& consumer) { return task_process_status::task_failed; } + + if (!genetlink_attribs.empty()) + { + if (!createGenetlinkHostifTable(trap_id_list)) + { + return task_process_status::task_failed; + } + } } else if (op == DEL_COMMAND) { @@ -715,10 +747,16 @@ task_process_status CoppOrch::processCoppRule(Consumer& consumer) vector trap_ids_to_reset; for (auto it : m_syncdTrapIds) { - if (it.second == m_trap_group_map[trap_group_name]) + if (it.second.trap_group_obj == m_trap_group_map[trap_group_name]) { trap_ids_to_reset.push_back(it.first); } + sai_status = sai_hostif_api->remove_hostif_trap(it.second.trap_obj); + if (sai_status != SAI_STATUS_SUCCESS) + { + SWSS_LOG_ERROR("Failed to remove trap object %ld", it.second.trap_obj); + return task_process_status::task_failed; + } } sai_attribute_t attr; @@ -756,15 +794,49 @@ task_process_status CoppOrch::processCoppRule(Consumer& consumer) return task_process_status::task_success; } +/* Program Sflow trap once we get sflow enable command */ +void CoppOrch::coppProcessSflow(Consumer &consumer) +{ + auto it = consumer.m_toSync.begin(); + + while (it != consumer.m_toSync.end()) + { + auto tuple = it->second; + string op = kfvOp(tuple); + + if (op == SET_COMMAND) + { + for (auto i : kfvFieldsValues(tuple)) + { + if (fvField(i) == "admin_state") + { + if (fvValue(i) == "enable") + { + enable_sflow_trap = true; + } + } + } + } + it = consumer.m_toSync.erase(it); + } +} + void CoppOrch::doTask(Consumer &consumer) { SWSS_LOG_ENTER(); + string table_name = consumer.getTableName(); if (!gPortsOrch->allPortsReady()) { return; } + if (table_name == CFG_SFLOW_TABLE_NAME) + { + coppProcessSflow(consumer); + return; + } + auto it = consumer.m_toSync.begin(); while (it != consumer.m_toSync.end()) { diff --git a/orchagent/copporch.h b/orchagent/copporch.h index 71f247cdc7..2fb7c9872a 100644 --- a/orchagent/copporch.h +++ b/orchagent/copporch.h @@ -25,27 +25,34 @@ const std::string copp_policer_action_red_field = "red_action"; const std::string copp_policer_action_yellow_field = "yellow_action"; // genetlink fields -const string copp_genetlink_name = "genetlink_name"; -const string copp_genetlink_mcgrp_name = "genetlink_mcgrp_name"; +const std::string copp_genetlink_name = "genetlink_name"; +const std::string copp_genetlink_mcgrp_name = "genetlink_mcgrp_name"; + +struct copp_trap_objects +{ + sai_object_id_t trap_obj; + sai_object_id_t trap_group_obj; +}; /* TrapGroupPolicerTable: trap group ID, policer ID */ typedef std::map TrapGroupPolicerTable; -/* TrapIdTrapGroupTable: trap ID, trap group ID */ -typedef std::map TrapIdTrapGroupTable; -/* TrapGroupHostIfMap: trap group ID to host interface ID */ +/* TrapIdTrapObjectsTable: trap ID, copp trap objects */ +typedef std::map TrapIdTrapObjectsTable; +/* TrapGroupHostIfMap: trap group ID, host interface ID */ typedef std::map TrapGroupHostIfMap; -/* TrapGroupHostIfMap: trap type to host table entry */ -typedef std::map TrapHostTblEntryMap; +/* TrapIdHostIfTableMap: trap type, host table entry ID*/ +typedef std::map TrapIdHostIfTableMap; class CoppOrch : public Orch { public: - CoppOrch(swss::DBConnector *db, std::string tableName); + CoppOrch(std::vector &tableConnectors); protected: object_map m_trap_group_map; + bool enable_sflow_trap; TrapGroupPolicerTable m_trap_group_policer_map; - TrapIdTrapGroupTable m_syncdTrapIds; + TrapIdTrapObjectsTable m_syncdTrapIds; TrapGroupHostIfMap m_trap_group_hostif_map; TrapIdHostIfTableMap m_trapid_hostif_table_map; @@ -65,8 +72,10 @@ class CoppOrch : public Orch sai_object_id_t getPolicer(std::string trap_group_name); - bool createGenetlinkHostIf(string trap_group_name, vector &hostif_attribs); - bool removeGenetlinkHostIf(string trap_group_name); + bool createGenetlinkHostIf(std::string trap_group_name, std::vector &hostif_attribs); + bool removeGenetlinkHostIf(std::string trap_group_name); + bool createGenetlinkHostifTable(std::vector &trap_id_name_list); + void coppProcessSflow(Consumer& consumer); virtual void doTask(Consumer& consumer); }; diff --git a/orchagent/orchdaemon.cpp b/orchagent/orchdaemon.cpp index 400d24f85a..02196a86be 100644 --- a/orchagent/orchdaemon.cpp +++ b/orchagent/orchdaemon.cpp @@ -115,7 +115,15 @@ bool OrchDaemon::init() 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); + + TableConnector confDbSflowTable(m_configDb, CFG_SFLOW_TABLE_NAME); + TableConnector appCoppTable(m_applDb, APP_COPP_TABLE_NAME); + + vector copp_table_connectors = { + confDbSflowTable, + appCoppTable + }; + CoppOrch *copp_orch = new CoppOrch(copp_table_connectors); TunnelDecapOrch *tunnel_decap_orch = new TunnelDecapOrch(m_applDb, APP_TUNNEL_DECAP_TABLE_NAME); VxlanTunnelOrch *vxlan_tunnel_orch = new VxlanTunnelOrch(m_applDb, APP_VXLAN_TUNNEL_TABLE_NAME); diff --git a/swssconfig/sample/00-copp.config.json b/swssconfig/sample/00-copp.config.json index e82242830c..a4742d79b1 100644 --- a/swssconfig/sample/00-copp.config.json +++ b/swssconfig/sample/00-copp.config.json @@ -55,5 +55,21 @@ "red_action":"drop" }, "OP": "SET" + }, + { + "COPP_TABLE:trap.group.sflow": { + "trap_ids": "sample_packet", + "trap_action":"trap", + "trap_priority":"1", + "queue": "2", + "meter_type":"packets", + "mode":"sr_tcm", + "cir":"1000", + "cbs":"1000", + "red_action":"drop", + "genetlink_name":"psample", + "genetlink_mcgrp_name":"packets" + }, + "OP": "SET" } ] From ac2780495155b3d64fc83c3ac940ba41e5319d10 Mon Sep 17 00:00:00 2001 From: dgsudharsan Date: Wed, 9 Oct 2019 17:53:31 -0700 Subject: [PATCH 5/7] Addressing code review comments --- orchagent/copporch.cpp | 54 +++++++++++------------------------------- orchagent/copporch.h | 2 +- 2 files changed, 15 insertions(+), 41 deletions(-) diff --git a/orchagent/copporch.cpp b/orchagent/copporch.cpp index 741c014e3c..4770c2eada 100644 --- a/orchagent/copporch.cpp +++ b/orchagent/copporch.cpp @@ -191,7 +191,7 @@ void CoppOrch::getTrapIdList(vector &trap_id_name_list, vector &trap_id_name_list) +bool CoppOrch::createGenetlinkHostIfTable(vector &trap_id_name_list) { SWSS_LOG_ENTER(); @@ -201,11 +201,11 @@ bool CoppOrch::createGenetlinkHostifTable(vector &trap_id_name_list) for (auto trap_id : trap_id_list) { - auto hostTbl_entry = m_trapid_hostif_table_map.find(trap_id); - sai_object_id_t trap_group_id = m_syncdTrapIds[trap_id].trap_group_obj; + auto host_tbl_entry = m_trapid_hostif_table_map.find(trap_id); - if (hostTbl_entry == m_trapid_hostif_table_map.end()) + if (host_tbl_entry == m_trapid_hostif_table_map.end()) { + sai_object_id_t trap_group_id = m_syncdTrapIds[trap_id].trap_group_obj; auto hostif_map = m_trap_group_hostif_map.find(trap_group_id); if (hostif_map != m_trap_group_hostif_map.end()) { @@ -628,38 +628,6 @@ task_process_status CoppOrch::processCoppRule(Consumer& consumer) } } } - if (!genetlink_attribs.empty()) - { - auto hostif_map = m_trap_group_hostif_map.find(m_trap_group_map[trap_group_name]); - - if (hostif_map != m_trap_group_hostif_map.end()) - { - for(sai_uint32_t idx = 0; idx < genetlink_attribs.size(); idx++) - { - auto hostif_attr = genetlink_attribs[idx]; - sai_status = sai_hostif_api->set_hostif_attribute(hostif_map->second, - &hostif_attr); - if(sai_status != SAI_STATUS_SUCCESS) - { - SWSS_LOG_ERROR("Failed to apply attribute[%d].id=%d to hostif for \ - trap group %s, error:%d", idx, hostif_attr.id, - trap_group_name.c_str(), sai_status); - - return task_process_status::task_ignore; - } - } - } - else - { - if (!genetlink_attribs.empty()) - { - if (!createGenetlinkHostIf(trap_group_name, genetlink_attribs)) - { - return task_process_status::task_failed; - } - } - } - } for (sai_uint32_t ind = 0; ind < trap_gr_attribs.size(); ind++) { @@ -715,7 +683,7 @@ task_process_status CoppOrch::processCoppRule(Consumer& consumer) if (!genetlink_attribs.empty()) { - if (!createGenetlinkHostifTable(trap_id_list)) + if (!createGenetlinkHostIfTable(trap_id_list)) { return task_process_status::task_failed; } @@ -804,6 +772,12 @@ void CoppOrch::coppProcessSflow(Consumer &consumer) auto tuple = it->second; string op = kfvOp(tuple); + /* + * Need to handled just 'config sflow enable' command to install the sflow trap group + * for the first time to ensure support of genetlink attributes. Rest of the fields or + * disable value or DEL command are not required to be handled + * + */ if (op == SET_COMMAND) { for (auto i : kfvFieldsValues(tuple)) @@ -826,14 +800,14 @@ void CoppOrch::doTask(Consumer &consumer) SWSS_LOG_ENTER(); string table_name = consumer.getTableName(); - if (!gPortsOrch->allPortsReady()) + if (table_name == CFG_SFLOW_TABLE_NAME) { + coppProcessSflow(consumer); return; } - if (table_name == CFG_SFLOW_TABLE_NAME) + if (!gPortsOrch->allPortsReady()) { - coppProcessSflow(consumer); return; } diff --git a/orchagent/copporch.h b/orchagent/copporch.h index 2fb7c9872a..c46ac6e62e 100644 --- a/orchagent/copporch.h +++ b/orchagent/copporch.h @@ -74,7 +74,7 @@ class CoppOrch : public Orch bool createGenetlinkHostIf(std::string trap_group_name, std::vector &hostif_attribs); bool removeGenetlinkHostIf(std::string trap_group_name); - bool createGenetlinkHostifTable(std::vector &trap_id_name_list); + bool createGenetlinkHostIfTable(std::vector &trap_id_name_list); void coppProcessSflow(Consumer& consumer); virtual void doTask(Consumer& consumer); From 1c2b8bfe832273c6a6f10a60973da47d5785954e Mon Sep 17 00:00:00 2001 From: dgsudharsan Date: Wed, 16 Oct 2019 13:35:10 -0700 Subject: [PATCH 6/7] removing copp config for sflow --- swssconfig/sample/00-copp.config.json | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/swssconfig/sample/00-copp.config.json b/swssconfig/sample/00-copp.config.json index a4742d79b1..e82242830c 100644 --- a/swssconfig/sample/00-copp.config.json +++ b/swssconfig/sample/00-copp.config.json @@ -55,21 +55,5 @@ "red_action":"drop" }, "OP": "SET" - }, - { - "COPP_TABLE:trap.group.sflow": { - "trap_ids": "sample_packet", - "trap_action":"trap", - "trap_priority":"1", - "queue": "2", - "meter_type":"packets", - "mode":"sr_tcm", - "cir":"1000", - "cbs":"1000", - "red_action":"drop", - "genetlink_name":"psample", - "genetlink_mcgrp_name":"packets" - }, - "OP": "SET" } ] From 9efc12866ba25acff07aec2bcae4023109437134 Mon Sep 17 00:00:00 2001 From: Garrick He Date: Thu, 17 Oct 2019 14:20:44 -0700 Subject: [PATCH 7/7] Change admin_state * Change admin_state from enable/disable to up/down to match SONiC YANG. Signed-off-by: Garrick He --- orchagent/copporch.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/orchagent/copporch.cpp b/orchagent/copporch.cpp index 4770c2eada..ab146b6983 100644 --- a/orchagent/copporch.cpp +++ b/orchagent/copporch.cpp @@ -784,7 +784,7 @@ void CoppOrch::coppProcessSflow(Consumer &consumer) { if (fvField(i) == "admin_state") { - if (fvValue(i) == "enable") + if (fvValue(i) == "up") { enable_sflow_trap = true; }