Open vidhya-rajan opened 1 year ago
vidhya-rajan
commented
1 year ago @dgsudharsan @venkatmahalingam @liat-grozovik @vadymhlushko-mlnx @hari-selvam Please let us know if the changes are good enough to upgrade hsflowd
Note- sflow container is not moved to bullseye
dgsudharsan
commented
1 year ago @jeff-yin @padmanarayana Can you please provide feedback?
jeff-yin
commented
1 year ago It looks fine to me but @padmanarayana also intends to provide feedback. I was trying to loop in Neil and Peter from InMon, but not sure how I can CC them in Github (username @sflow).
sflow
commented
1 year ago @jeff-yin notifications to sflow go to me (Neil). I'm not sure I understand the problem yet. It sounds as though the compiling system reports kernel >= 5.4 while its file system is somehow from an older Linux release, so the expected netlink dropmon include-file definitions are not present. Did I get that right? And if so, is that a temporary problem?
vidhya-rajan
commented
1 year ago Yes your understanding on the problem is right. Build environment (buster) has older kernel version which will not have the required headers . Current SONIC has a local header file patch fix to address this issue . In order to enable this patch ENABLE_SFLOW_DROPMON = y has to be set.
sflow
commented
1 year ago We checked in a small change to hsflowd and released v2.0.42-1. The build is no longer conditional on the kernel version. Instead mod_dropmon.c can now be compiled successfully on older distributions. However it is not enabled and configured by default. That is expected to happen in /etc/hsflowd.conf. This is worth discussing here because the patch above appears to be forcing the dropmon module to be enabled, whether it appears in /etc/hsflowd.conf or not (those changes in hsflowd.c). Is that what you want for this SONiC profile? I think it is probably OK to do that. The module can listen for hw drops, but nothing will happen unless the switch ASIC/driver actually sends to that netlink channel. We didn't enable the dropmon module by default because not all ASIC/driver implementations report drops yet... but if that is changing soon then maybe we should revisit that?
vidhya-rajan
commented
1 year ago Thank you. We forcibly enabled Dropmon module in this patch in order to apply the workaround patch for headers. With the changes in v2.0.42-1 we see that the compilation is successful . So enabling dropmon by default is no more required and we can continue to disable by default. Thanks again for the immediate fix.
As highlighted by you not all ASICs may support dropmon so the default can still be retained as disabled. However the earlier patch Updated-0001-sflow-enabled-drop-monitor-support-for-SONiC.patch.txt has to be enhanced to be in sync with the latest versions of host flow where group attribute of dropmon is deprecated. 0001-dropmon-workaround-created-local-copy-of-linux-net_dropmon.patch => Once host-sflow is upgraded to v2.0.42-1 this patch will be removed in SONIC
padmanarayana
commented
1 year ago Thanks @sflow for the v2.0.42-1 release! @vidhya-rajan the plan looks good.
Description
host-sflow have few critical fixes for sample flow and SONIC has to be upgraded to the latest version to incorporate those fixes
Steps to reproduce the issue:
Upgrade SONIC to host sflow 2.41-1 latest version
Describe the results you received:
Upgrade will result in compilation issues mod_dropmon.c:368:22: error: 'NET_DM_ATTR_HW_DROPS' undeclared (first use in this function); did you mean 'NET_DM_CMD_STOP'? attr2.nla_type = NET_DM_ATTR_HW_DROPS;
Describe the results you expected:
Compilation of SONIC should be successfull
Additional information you deem important (e.g. issue happens only occasionally):
Below files have been modified for successful compilation in order to upgrade host-sflow to latest version
Please let us know if the changes are good enough to upgrade hsflowd