vmware / go-ipfix

An ipfix library in Go
Other
39 stars 27 forks source link

Fix how decoding errors are handled for UDP clients in collector #352

Closed antoninbas closed 3 weeks ago

antoninbas commented 3 weeks ago

Previously, in case of a decoding error (e.g., an unknown template ID in a data set), the goroutine responsible for decoding messages would be stopped, but the client would not be deleted. This would lead to a deadlock as subsequent messages could not be written to the packetChan channel for the client, effectively breaking the UDP collecting process completely. To avoid this issue, we log an error when decoding fails, but keep the client alive. Note that for IPFIX over UDP, there is no way to notify a single UDP client of an error (with TCP, we can close the connection if we want). This is also why the template refresh mechanism exists.

We also fix possible race conditions in the UDP collecting process: all access to the clients map should be protected by locking the mutex.

antoninbas commented 3 weeks ago

This actually supersedes https://github.com/vmware/go-ipfix/pull/277, which was never merged (because of a CLA issue?)

codecov[bot] commented 3 weeks ago

Codecov Report

Attention: Patch coverage is 94.73684% with 3 lines in your changes missing coverage. Please review.

Project coverage is 70.37%. Comparing base (79a2702) to head (e474105). Report is 3 commits behind head on main.

Files Patch % Lines
pkg/collector/udp.go 92.68% 3 Missing :warning:
Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/vmware/go-ipfix/pull/352/graphs/tree.svg?width=650&height=150&src=pr&token=448A94JZ59&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=vmware)](https://app.codecov.io/gh/vmware/go-ipfix/pull/352?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=vmware) ```diff @@ Coverage Diff @@ ## main #352 +/- ## =========================================== + Coverage 50.65% 70.37% +19.71% =========================================== Files 9 20 +11 Lines 1289 3048 +1759 =========================================== + Hits 653 2145 +1492 - Misses 516 731 +215 - Partials 120 172 +52 ``` | [Flag](https://app.codecov.io/gh/vmware/go-ipfix/pull/352/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=vmware) | Coverage Δ | | |---|---|---| | [integration-tests](https://app.codecov.io/gh/vmware/go-ipfix/pull/352/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=vmware) | `53.56% <89.47%> (+2.90%)` | :arrow_up: | | [unit-tests](https://app.codecov.io/gh/vmware/go-ipfix/pull/352/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=vmware) | `69.42% <94.73%> (?)` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=vmware#carryforward-flags-in-the-pull-request-comment) to find out more. | [Files](https://app.codecov.io/gh/vmware/go-ipfix/pull/352?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=vmware) | Coverage Δ | | |---|---|---| | [pkg/collector/process.go](https://app.codecov.io/gh/vmware/go-ipfix/pull/352?src=pr&el=tree&filepath=pkg%2Fcollector%2Fprocess.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=vmware#diff-cGtnL2NvbGxlY3Rvci9wcm9jZXNzLmdv) | `85.38% <ø> (+14.43%)` | :arrow_up: | | [pkg/collector/tcp.go](https://app.codecov.io/gh/vmware/go-ipfix/pull/352?src=pr&el=tree&filepath=pkg%2Fcollector%2Ftcp.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=vmware#diff-cGtnL2NvbGxlY3Rvci90Y3AuZ28=) | `70.32% <100.00%> (+4.89%)` | :arrow_up: | | [pkg/collector/udp.go](https://app.codecov.io/gh/vmware/go-ipfix/pull/352?src=pr&el=tree&filepath=pkg%2Fcollector%2Fudp.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=vmware#diff-cGtnL2NvbGxlY3Rvci91ZHAuZ28=) | `75.72% <92.68%> (+7.59%)` | :arrow_up: | ... and [17 files with indirect coverage changes](https://app.codecov.io/gh/vmware/go-ipfix/pull/352/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=vmware)