Conversation
dgsudharsan
force-pushed
the
copp_sflow
branch
from
a50e717 to
c98babb
Compare
dgsudharsan
force-pushed
the
copp_sflow
branch
from
c98babb to
0d774ab
Compare
|
retest this please |
undoing revert indentation Code review fixes Addressing code review comments Latest copp changes based on headers
Addressing code review comments
dgsudharsan
force-pushed
the
copp_sflow
branch
from
43c91a0 to
59d93d9
Compare
UT Fixes Fixing build Build fix
dgsudharsan
force-pushed
the
copp_sflow
branch
from
59d93d9 to
42a289f
Compare
| return; | ||
| } | ||
|
|
||
| if (table_name == CFG_SFLOW_TABLE_NAME) |
There was a problem hiding this comment.
I think this can be done irrespective of allPortsReady. It can avoid the wait.
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Please add an else case and log NOTICE stating 'not implemented'
There was a problem hiding this comment.
Will add comment, since a log will be misleading
|
|
||
| if (!genetlink_attribs.empty()) | ||
| { | ||
| if (!createGenetlinkHostifTable(trap_id_list)) |
| 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"); |
There was a problem hiding this comment.
Can this have other traps in list that must installed?
There was a problem hiding this comment.
No. Only sflow can be supported in this trap group
| { | ||
| 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()) |
There was a problem hiding this comment.
Do we have a use-case for setting Genetlink host if attribute?
There was a problem hiding this comment.
No. Will remove this code
| { | ||
| trap_ids_to_reset.push_back(it.first); | ||
| } | ||
| sai_status = sai_hostif_api->remove_hostif_trap(it.second.trap_obj); |
There was a problem hiding this comment.
Why this new code? I mean it looks generic.
There was a problem hiding this comment.
This logic is added to clean up hostif trap during delete case, which was missing in the existing logic
| } | ||
| } | ||
|
|
||
| bool CoppOrch::createGenetlinkHostifTable(vector<string> &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; |
|
|
||
| for (auto trap_id : trap_id_list) | ||
| { | ||
| auto hostTbl_entry = m_trapid_hostif_table_map.find(trap_id); |
There was a problem hiding this comment.
Use variable style consistently
|
retest this please |
| "genetlink_name":"psample", | ||
| "genetlink_mcgrp_name":"packets" | ||
| }, | ||
| "OP": "SET" |
There was a problem hiding this comment.
suggest to remove this configuration, so that we can merge other parts of the PR.
There was a problem hiding this comment.
Hi Guohan,
Now the changes are made in such a way that only when "config sflow enable" is issued in platform, these configurations would be played. Hence in platforms which doesn't support sflow, these configurations would not be played at all
* Change admin_state from enable/disable to up/down to match SONiC YANG. Signed-off-by: Garrick He <[email protected]>
|
retest this please |
|
retest this please |
2 similar comments
|
retest this please |
|
retest this please |
) Create environment files for SONiC immutable attribute. The file will reside in the incoming image dir under sonic-config dir. Incoming image will then move the file to /etc/sonic. The file will be used to avoid calls into cfggen for those attributes. signed-off-by: Tamer Ahmed <[email protected]>
5 participants
What I did
Copp orch changes for programming genetlink attributes defined in opencomputeproject/SAI#936 and also defining trap group for SFLOW
Why I did it
As part of SFLOW changes in host interface are required to support genetlink.
How I verified it
Details if related
PLEASE DO NOT MERGE UNTIL SAI Support is added.