[Bug Fix] Add NF Cleanup after Critical Errors#131
Conversation
CI MessageYour results will arrive shortly |
onvm
left a comment
onvm
left a comment
There was a problem hiding this comment.
In response to PR creation
CI Message
Run successful see results:
✔️ PR submitted to develop branch
✔️ Speed tester performance check passed
❌ Linter Failed (please fix style errors)
[Results from nimbnode30]
- Median TX pps for Speed Tester: 39335955
- Performance rating - 112.39% (compared to 35000000 average)
Linter Output
examples/speed_tester/speed_tester.c:331: Missing space before { [whitespace/braces] [5]
examples/speed_tester/speed_tester.c:511: Missing space before { [whitespace/braces] [5]
Total errors found: 2
|
@onvm I fixed it |
CI MessageYour results will arrive shortly |
onvm
left a comment
onvm
left a comment
There was a problem hiding this comment.
@onvm I fixed it
CI Message
Run successful see results:
✔️ PR submitted to develop branch
✔️ Speed tester performance check passed
✔️ Linter passed
[Results from nimbnode30]
- Median TX pps for Speed Tester: 39317033
- Performance rating - 112.33% (compared to 35000000 average)
|
@onvmTest test this |
|
@onvmTest again |
|
@onvmTest data here |
|
@onvmTest last time rn |
CI MessageThe current user is true |
CI MessageYour results will arrive shortly |
onvm
left a comment
onvm
left a comment
There was a problem hiding this comment.
@onvmTest last time rn
CI Message
Run successful see results:
✔️ PR submitted to develop branch
✔️ Linter passed
koolzz
changed the title
|
@onvmTest will you work |
|
@onvmTest I should have saved the file |
CI MessageYour results will arrive shortly |
onvm
left a comment
onvm
left a comment
There was a problem hiding this comment.
@onvmTest I should have saved the file
CI Message
Run successful see results:
✔️ PR submitted to develop branch
✔️ Linter passed
|
@onvm no way |
CI MessageYour results will arrive shortly |
|
@onvm testing now |
CI MessageYour results will arrive shortly |
onvm
left a comment
onvm
left a comment
There was a problem hiding this comment.
@onvm testing now
CI Message
Run successful see results:
✔️ PR submitted to develop branch
✔️ Speed tester performance check passed
✔️ Linter passed
[Results from nimbnode30]
- Median TX pps for Speed Tester: 39313593
- Performance rating - 112.32% (compared to 35000000 average)
4 participants
Speed test had some outstanding errors where we
rte_exitwithout cleaning up contextSummary:
There were a few places where we would exit because of an error after registering the nf with the manager, but didn't clean up properly. This would cause the manager to think the nf was still running, even when it already exited.
Usage:
Speed test, when pcap was enabled, if you input an invalid pcap file, this would break manager. Also, when not enough packets could be created for speed_tester this happens. These are two odd edge cases that we've caught in the past, but didn't remember to verify after changing how we exit NFs. There might be other places, but these were the only other ones I found, and couldn't find other relevant ones in other NFs.
Merging notes:
TODO before merging :
Test Plan:
First test that the functionality works and doesn't break on the test cases. Also, I checked if other NFs had similar problems (rte_exit was called but
onvm_nflib_stop(nf_local_ctx)was not). I did not see any places in other NFs like this, but I might have missed them. Something to keep in mind.Review:
@koolzz @dennisafa
(optional) Subscribers: << @-mention people who probably care about these changes >>