NF core reassignment on termination #87

koolzz · 2019-03-21T01:14:41Z

Adds functionality to onvm_mgr to reassign cores on NF shutdown.

Summary:

Reallocates a NF to another core when another NF shuts down and a better core allocation is possible.
This is a part of multiple changes regarding the autoscaling and core managing improvments for ONVM, working with @ratnadeepb on these.

Usage:
Run onvm_mgr with 2 cores for NFs, f.e:
./go.sh 0,1,2 0 0x60
Run basic monitor (or any other NF really)
./go.sh 2
Run basic monitor in shared more (or any other NF really)
./go.sh -l 0 -- -r 1 -s --
Run another basic monitor in shared more (or any other NF really)
./go.sh -l 0 -- -r 1 -s --
*Then Ctrl-C the first NF (the one not running in shared mode)
The second NF should now be reallocated to the core that the first NF was running on.

This PR includes
Resolves issues
Breaking API changes
Internal API changes
Usability improvements
Bug fixes
New functionality	👍
New NF/onvm_mgr args
Changes to starting NFs
Dependency updates
Web stats updates

Merging notes:

Dependencies: None

TODO before merging :

PR is ready for review

Test Plan:

Figure out if this can break anything.

Review:

Review checklist:

Sanity checks, assigned to @dennisafa @rskennedy
- Run make in /onvm and /examples directories
- Test the new functionality
Code style, assigned to @dennisafa @rskennedy
- Run linter
- Check everything style related
Code design, assigned to @dennisafa @rskennedy
- Check for memory leaks
- Check that code is reusable
- Code is clean, functions are concise
- Verify that edge cases properly handled
Performance, assigned to @dennisafa @rskennedy
- Run Speed Tester NF, report performance.
- Run pktgen, report performance (not really required)
Documentation, assigned to @dennisafa @rskennedy
- Check if the new changes are well documented, in both code and READMEs

onvm · 2019-03-21T01:14:49Z

In response to PR creation

CI Message

Your results will arrive shortly

koolzz · 2019-03-21T19:03:56Z

PR review assigned to @dennisafa @rskennedy

…core_reassignment

koolzz · 2019-03-23T06:48:09Z

@onvm hello my old friend

onvm · 2019-03-23T06:48:16Z

@onvm hello my old friend

CI Message

Your results will arrive shortly

onvm · 2019-03-23T06:55:07Z

@onvm hello my old friend

CI Message

Run successful see results:
[Results from nimbnode30]
Median TX pps for Speed Tester: 34822438

Linter Passed

dennisafa

just some style nits

dennisafa · 2019-03-23T14:31:21Z

onvm/onvm_nflib/onvm_nflib.c

@@ -585,7 +585,14 @@ onvm_nflib_handle_msg(struct onvm_nf_msg *msg, __attribute__((unused)) struct on
                        RTE_LOG(INFO, APP, "Received scale message...\n");
                        onvm_nflib_scale((struct onvm_nf_scale_info *)msg->msg_data);
                        break;
-                case MSG_NOOP:
+                case MSG_CHANGE_CORE:
+                        RTE_LOG(INFO, APP, "Recieved relocation message...\n"); 


typo: Received

dennisafa · 2019-03-23T14:32:27Z

onvm/onvm_nflib/onvm_threading.c

+onvm_threading_find_nf_to_reassign_core(uint16_t candidate_core, struct core_status *cores) {
+        int i;
+        int candidate_nf_id, most_used_core;
+        int max_nfs_per_core;


maybe combine all these into one?
int i, candidate_nf_id, most_used_core, max_nfs_per_core;

dennisafa · 2019-03-23T14:33:39Z

onvm/onvm_nflib/onvm_threading.h

+int
+onvm_threading_find_nf_to_reassign_core(uint16_t candidate_core, struct core_status *cores);
+
+


style nit: remove line

dennisafa

Sanity checks
- Making in ONVM and examples works properly. Core is reassigned upon exiting NF as test plan described.
Code style
- Minor style nits
Code design
- Variables are understandably named, design is clean and concise. No recommended design changes.
Performance, assigned to @dennisafa @rskennedy
- Speed_tester runs properly at avg speed of 56m pps.
Documentation
- onvm_threading.h is documented properly.

dennisafa · 2019-03-27T18:18:44Z

onvm/onvm_mgr/onvm_nf.c

+ * Output : an error code
+ *
+ */
+inline int 


95: Line ends in whitespace.

dennisafa · 2019-03-27T18:19:21Z

onvm/onvm_mgr/onvm_nf.c

+
+inline int onvm_nf_relocate_nf(uint16_t dest, uint16_t new_core) {
+        uint16_t *msg_data;
+


314: Line ends in whitespace.

dennisafa · 2019-03-27T18:20:16Z

onvm/onvm_nflib/onvm_nflib.c

@@ -585,7 +585,14 @@ onvm_nflib_handle_msg(struct onvm_nf_msg *msg, __attribute__((unused)) struct on
                        RTE_LOG(INFO, APP, "Received scale message...\n");
                        onvm_nflib_scale((struct onvm_nf_scale_info *)msg->msg_data);
                        break;
-                case MSG_NOOP:
+                case MSG_CHANGE_CORE:
+                        RTE_LOG(INFO, APP, "Received relocation message...\n"); 


589: Line ends in whitespace.

dennisafa · 2019-03-27T18:20:31Z

onvm/onvm_nflib/onvm_nflib.c

-                case MSG_NOOP:
+                case MSG_CHANGE_CORE:
+                        RTE_LOG(INFO, APP, "Received relocation message...\n"); 
+                        RTE_LOG(INFO, APP, "Moving NF to core %d\n", *(uint16_t *)msg->msg_data); 


590: Line ends in whitespace.

koolzz · 2019-03-28T14:47:21Z

@dennisafa implemented requested changes, approve if everything else is good.

koolzz · 2019-03-28T14:47:50Z

@rskennedy give this a review when you can

koolzz · 2019-04-01T03:28:01Z

The more I though about this we should create a macro for it. This is optimization that might not always be wanted.

…core_reassignment Conflicts: onvm/onvm_mgr/onvm_nf.c onvm/onvm_nflib/onvm_msg_common.h

koolzz · 2019-04-01T17:56:00Z

@onvm updated.

onvm · 2019-04-01T17:56:08Z

@onvm updated.

CI Message

Your results will arrive shortly

onvm · 2019-04-01T18:03:02Z

@onvm updated.

CI Message

Run successful see results:
[Results from nimbnode30]
Median TX pps for Speed Tester: 35204966

Linter Passed

koolzz · 2019-04-01T18:04:56Z

@dennisafa does introducing the macro make sense?

dennisafa · 2019-04-01T18:55:59Z

@dennisafa does introducing the macro make sense?

Yea, I'm a fan of the macros. It makes sense to be able to enable or disable them (like ONVM_NF_SHUTDOWN_CORE_REASSIGNMENT) based on what your purposes may be.

Edit: I'll give it another test for sanity check

koolzz · 2019-05-11T21:01:53Z

Will update with the new config approach when shared cpu changes are merged

…core_reassignment Conflicts: onvm/onvm_mgr/onvm_nf.c onvm/onvm_nflib/onvm_msg_common.h onvm/onvm_nflib/onvm_nflib.c onvm/onvm_nflib/onvm_threading.h

kevindweb · 2019-06-24T23:33:53Z

@onvm should be put in queue

onvm · 2019-06-24T23:33:54Z

@onvm should be put in queue

CI Message

Another CI run in progress, adding request to the end of the list

kevindweb · 2019-06-24T23:33:57Z

@onvm hopefully this doesn't run

onvm · 2019-06-24T23:33:58Z

@onvm hopefully this doesn't run

CI Message

Duplicate request already waiting, ignoring message

onvm · 2019-06-24T23:40:20Z

@onvm should be put in queue

CI Message

Your results will arrive shortly

onvm

@onvm should be put in queue

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: 39265985
Performance rating - 112.19% (compared to 35000000 average)

kevindweb · 2019-07-26T22:20:47Z

@onvm not sure why nfd isn't working

onvm · 2019-07-26T22:20:48Z

@onvm not sure why nfd isn't working

CI Message

Your results will arrive shortly

onvm · 2019-07-26T22:26:38Z

@onvm not sure why nfd isn't working

CI Message

Error: ERROR: Failed to post results to GitHub

kevindweb · 2019-07-26T22:41:58Z

@onvm this should be better now

onvm · 2019-07-26T22:41:59Z

@onvm this should be better now

CI Message

Your results will arrive shortly

onvm · 2019-07-26T22:49:41Z

@onvm this should be better now

CI Message

Error: ERROR: Failed to post results to GitHub

kevindweb · 2019-07-26T23:46:20Z

@onvm test

kevindweb · 2019-07-26T23:47:30Z

@onvm now really test

onvm · 2019-07-26T23:47:31Z

@onvm now really test

CI Message

Your results will arrive shortly

onvm · 2019-07-26T23:53:46Z

@onvm now really test

CI Message

Error: ERROR: Failed to fetch results from nimbnode17

kevindweb · 2019-07-27T00:15:29Z

@onvm please send 0

onvm · 2019-07-27T00:15:30Z

@onvm please send 0

CI Message

Your results will arrive shortly

onvm

@onvm please send 0

CI Message

Run successful see results:
✔️ PR submitted to develop branch
❌ PR drops Pktgen performance below minimum requirement
✔️ Speed Test performance check passed
✔️ mTCP performance check passed
✔️ Linter passed

[Results from nimbnode17]

Median TX pps for Speed Tester: 41231422
Performance rating - 103.08% (compared to 40000000 average)
Median TX pps for Pktgen: 6420024
Performance rating - 64.20% (compared to 10000000 average)
Time (ms) per request for mTCP and Apache Benchmark: 0.235000
Performance rating - 102.17% (compared to 0.230000 average)

…core_reassignment

koolzz · 2019-08-02T19:01:33Z

@onvm
Updated with the latest version

onvm · 2019-08-02T23:19:24Z

Testing

CI Message

Your results will arrive shortly

onvm

Let us see

CI Message

Run successful see results:
✔️ PR submitted to develop branch
✔️ Pktgen performance check passed
✔️ Speed Test performance check passed
❌ PR drops mTCP performance below minimum requirement
✔️ Linter passed

[Results from nimbnode17]

Median TX pps for Speed Tester: 40206604
Performance rating - 100.52% (compared to 40000000 average)
Median TX pps for Pktgen: 10184821
Performance rating - 101.85% (compared to 10000000 average)
Time (ms) per request for mTCP and Apache Benchmark: 0.075000
Performance rating - 32.61% (compared to 0.230000 average)

kevindweb · 2019-08-03T00:22:10Z

I'll fix that mTCP thing (not a bug apache benchmark just went faster), I'm just trying to make sure pktgen is working

onvm · 2019-08-06T00:42:06Z

Testing

CI Message

Error: ERROR: Failed to copy ONVM files to nimbnode17

onvm · 2019-08-06T00:43:07Z

Testing

CI Message

Error: ERROR: Script failed on nimbnode17

koolzz added 3 commits March 19, 2019 14:55

Add core reassignment on NF shutdown

e2a6c8f

Fix core allocation order, cleanup

056d688

Better naming, fix typos

be8f689

This comment has been minimized.

Sign in to view

koolzz added the enhancement 🚀 label Mar 22, 2019

Merge branch 'develop' of https://github.com/sdnfv/openNetVM into nf_…

446e56a

…core_reassignment

dennisafa requested changes Mar 23, 2019

View reviewed changes

koolzz added 2 commits March 23, 2019 13:06

Fix minor style changes

2873ef3

Fix typo

2d6d43b

dennisafa requested changes Mar 27, 2019

View reviewed changes

Fix extra whitespace

0310540

dennisafa previously approved these changes Mar 28, 2019

View reviewed changes

Merge branch 'develop' of https://github.com/sdnfv/openNetVM into nf_…

abc51cf

…core_reassignment Conflicts: onvm/onvm_mgr/onvm_nf.c onvm/onvm_nflib/onvm_msg_common.h

koolzz mentioned this pull request Apr 2, 2019

Comprehensive list of enhancements #91

Open

15 tasks

koolzz added 2 commits May 24, 2019 11:28

Merge branch 'develop' of https://github.com/sdnfv/openNetVM into nf_…

430970c

…core_reassignment Conflicts: onvm/onvm_mgr/onvm_nf.c onvm/onvm_nflib/onvm_msg_common.h onvm/onvm_nflib/onvm_nflib.c onvm/onvm_nflib/onvm_threading.h

WIP with rearch pr

2327536

onvm approved these changes Jun 24, 2019

View reviewed changes

onvm suggested changes Jul 27, 2019

View reviewed changes

koolzz added this to the ONVM v19.07 milestone Jul 31, 2019

Merge branch 'develop' of https://github.com/sdnfv/openNetVM into nf_…

5a7cd9d

…core_reassignment

Disable core reallocation by default

d62a57d

koolzz dismissed dennisafa’s stale review via d62a57d August 2, 2019 19:47

onvm suggested changes Aug 3, 2019

View reviewed changes

koolzz merged commit e217d71 into sdnfv:develop Aug 3, 2019

		int
		onvm_threading_find_nf_to_reassign_core(uint16_t candidate_core, struct core_status *cores);


		inline int onvm_nf_relocate_nf(uint16_t dest, uint16_t new_core) {
		uint16_t *msg_data;

NF core reassignment on termination #87

NF core reassignment on termination #87

Conversation

koolzz commented Mar 21, 2019 • edited Loading

Summary:

Merging notes:

Test Plan:

Review:

Review checklist:

onvm commented Mar 21, 2019

CI Message

This comment has been minimized.

koolzz commented Mar 21, 2019

koolzz commented Mar 23, 2019

onvm commented Mar 23, 2019

CI Message

onvm commented Mar 23, 2019

CI Message

Linter Passed

dennisafa left a comment

Choose a reason for hiding this comment

dennisafa Mar 23, 2019

Choose a reason for hiding this comment

dennisafa Mar 23, 2019

Choose a reason for hiding this comment

dennisafa Mar 23, 2019

Choose a reason for hiding this comment

dennisafa left a comment

Choose a reason for hiding this comment

dennisafa Mar 27, 2019

Choose a reason for hiding this comment

dennisafa Mar 27, 2019

Choose a reason for hiding this comment

dennisafa Mar 27, 2019

Choose a reason for hiding this comment

dennisafa Mar 27, 2019

Choose a reason for hiding this comment

koolzz commented Mar 28, 2019

koolzz commented Mar 28, 2019

koolzz commented Apr 1, 2019

koolzz commented Apr 1, 2019

onvm commented Apr 1, 2019

CI Message

onvm commented Apr 1, 2019

CI Message

Linter Passed

koolzz commented Apr 1, 2019

dennisafa commented Apr 1, 2019 • edited Loading

koolzz commented May 11, 2019

kevindweb commented Jun 24, 2019

onvm commented Jun 24, 2019

CI Message

kevindweb commented Jun 24, 2019

onvm commented Jun 24, 2019

CI Message

onvm commented Jun 24, 2019

CI Message

onvm left a comment

Choose a reason for hiding this comment

CI Message

kevindweb commented Jul 26, 2019

onvm commented Jul 26, 2019

CI Message

onvm commented Jul 26, 2019

CI Message

kevindweb commented Jul 26, 2019

onvm commented Jul 26, 2019

CI Message

onvm commented Jul 26, 2019

CI Message

kevindweb commented Jul 26, 2019

kevindweb commented Jul 26, 2019

onvm commented Jul 26, 2019

CI Message

onvm commented Jul 26, 2019

CI Message

kevindweb commented Jul 27, 2019

onvm commented Jul 27, 2019

CI Message

onvm left a comment

koolzz commented Mar 21, 2019 •

edited

Loading

dennisafa commented Apr 1, 2019 •

edited

Loading