Refactors Advanced Rings API/Examples #124

koolzz · 2019-05-26T23:13:14Z

Currently the advanced ring support in onvm is not clean, this pr attempts to resolve this.

Summary:

This PR resolves such issues as:

Removing unused APIs
Removing support for APIs written for advanced rings (see advanced rings scaling) that would never actually be used by the developers, because if developers are using advanced rings the assumption is they will also want to manage their own threads etc.
Remove Advanced Rings mode from the Speed tester NF
Cleanup and redo adv ring mode in the Scaling NF, the mode now does its own pthread creation instead of relying on onvm scaling APIs. Also make a clear separation between default and advanced ring mode.
Because of the adv rings changes some internal nflib APIs were exposed to the NF (onvm_nflib_start_nf, onvm_nflib_init_nf_init_cfg, onvm_nflib_inherit_parent_init_cfg)

Usage:

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: merge after Consolidates All NF Structs #117

TODO before merging :

PR is ready for review

Test Plan:

Still working on it but primarily we will need to test the scaling NF

Review:

(optional) << @-mention people who should review these changes >>

(optional) Subscribers: << @-mention people who probably care about these changes >>

This code introduces **EXPERIMENTAL** support for NFs that can efficiently run on **shared** CPUs. NFs wait on semaphores when idle and are signaled by the manager when new packets arrive. Once the NF is in wake state, no additional notifications will be sent until it goes back to sleep. All code changes are featurized using INTERRUPT_SEM macro. See the ONVM README.md file for more information and warnings. Commit log: * merges rebased to master with sdnfv#147 * reverting num_clients update. * replace tabs with spaces * Readme describing experimental shared CPU support * Add usage and known limitations for shared CPU code

…fv#185) Fixes a race condition in NF instance ID assignment. Before, if two NFs were started at the same time, they would be assigned the same instance ID. This change modifies logic in `onvm_nf_next_instance_id` to avoid assigning the same ID to two starting NFs. Commit log: * Bug Fix: Fixes instance ID assignment race condition - Fixes sdnfv#178 - Changed incrementation of next_instance_id in onvm_mgr/onvm_nf.c such that the ID can't be reused before an NF becomes active.

…ith onvm_nf struct

…nd flow_tracker). Also did some small style changes in some other documentation in onvm_nflib.h

Conflicts: README.md onvm/onvm_mgr/onvm_init.c onvm/onvm_mgr/onvm_init.h onvm/onvm_mgr/onvm_mgr.h onvm/onvm_mgr/onvm_nf.c onvm/onvm_mgr/onvm_stats.c onvm/onvm_nflib/onvm_common.h onvm/onvm_nflib/onvm_nflib.c onvm/onvm_nflib/onvm_nflib.h

…experimental/shared_cpu

- Some stats were unused and broken(?) nuked them - added stats that are possible usefull to stdout (slightly ugly)

…multithread Conflicts: examples/speed_tester/speed_tester.c

- Keeping onvm_nf_info small, moved vars to onvm_nf - Added a callback function for the scaling child thread - Cleaned up code

- Adds ability to scale NFs with advanced rings - Adds a setup function for NFs - More docs and cleanup

- Moving nf_info as passed arg to packet_handler -> allows for spawned NF to access their own nf_info - Launching new NFs now accepts arguments for packet_handler function, setup function, callback function, advanced rings function - Add a data pointer to onvm_nflib_info to maintain state - Working on a scaling example (currently testing code, not in usable state) will showcase different features of spawning NFs

- This is a major change, a lot of nflib initlization is refactored to allow spawned NFs to launch with their own service id - There is a bug when the spawn a lot of new NFs we can no longer retrieve the nf_info from nf_info_mp, this needs to be fixed as it breaks stuff

- Fixed the nf_info bug where the mempool would get messed up as children weren't cleared properly. - Updated the example scaling NF - Removed most of the debugging code

Cleaned up messy bits, replacing macro branches with if statements, encapsulating code in functions, and getting rid of unnecessary code.

- Style fixes - Removing debug code - Making code prettier - Adding better docs - Implementing requested changes

…o_adv_rings Conflicts: examples/scaling_example/scaling.c examples/speed_tester/speed_tester.c onvm/onvm_nflib/onvm_common.h onvm/onvm_nflib/onvm_nflib.c onvm/onvm_nflib/onvm_nflib.h

koolzz · 2019-05-31T14:22:05Z

@onvm linty lint

onvm · 2019-05-31T14:22:15Z

@onvm linty lint

CI Message

Your results will arrive shortly

onvm

@onvm linty lint

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

Linter Output

examples/scaling_example/scaling.c:161: Lines should be <= 120 characters long [whitespace/line_length] [5]
examples/scaling_example/scaling.c:309: Line ends in whitespace. Consider deleting these extra spaces. [whitespace/end_of_line] [4]
Total errors found: 2

koolzz · 2019-05-31T19:21:35Z

@onvm gogogo

onvm · 2019-05-31T19:21:42Z

@onvm gogogo

CI Message

Your results will arrive shortly

onvm

@onvm gogogo

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

koolzz · 2019-05-31T19:30:55Z

@dennisafa @kevindweb Check this out if you have time, if shows a clean separation between adv and normal rings(at least it should so comment if this needs any changes).

@nks5295 This is good for review now. Updated the description

kevindweb

It's PR review day, for me. Success! 13555168mpps for Pktgen, amazing 51877024 with 1 speed tester, dropped to 49639520 and 43844704 after a second was added. 19094944+ when running 2 speed_testers from 1 -d 4 and 4 -d 1. Approved 👍

koolzz · 2019-06-02T20:44:58Z

@onvm comrade проверь код

onvm · 2019-06-02T20:45:05Z

@onvm comrade проверь код

CI Message

Your results will arrive shortly

onvm

@onvm comrade проверь код

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

…o_adv_rings Conflicts: examples/scaling_example/scaling.c examples/speed_tester/speed_tester.c onvm/onvm_nflib/onvm_nflib.c

koolzz · 2019-06-03T04:21:16Z

@onvm go for it

onvm · 2019-06-03T04:21:18Z

@onvm go for it

CI Message

Your results will arrive shortly

onvm

@onvm go for 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: 38140314
Performance rating - 108.97% (compared to 35000000 average)

koolzz · 2019-06-03T04:50:27Z

@nks5295 ready for your review 📝

sameergk and others added 30 commits February 13, 2017 13:26

partial refactoring of api w accordance of nf_info

df6fbb7

onvm mgr compiles

e1ffe07

flow table compiles -- all examples compile

dee954c

changed onvm_get_rx and onvm_get_tx to get rx_q and tx_q associated w…

fb99a9c

…ith onvm_nf struct

resolve merge conflicts

0bff75a

Did requested changes. Also updated newer NFs (such as arp_response a…

8886eaf

…nd flow_tracker). Also did some small style changes in some other documentation in onvm_nflib.h

Corrected some incorrect references to nflib functions in NF_Dev.md

8a72816

removed typing error

a1dccf2

Update shared_cpu branch

315722a

Conflicts: README.md onvm/onvm_mgr/onvm_init.c onvm/onvm_mgr/onvm_init.h onvm/onvm_mgr/onvm_mgr.h onvm/onvm_mgr/onvm_nf.c onvm/onvm_mgr/onvm_stats.c onvm/onvm_nflib/onvm_common.h onvm/onvm_nflib/onvm_nflib.c onvm/onvm_nflib/onvm_nflib.h

Minor cleanup

71a07dc

Merge branch 'master' of https://github.com/sdnfv/openNetVM-dev into …

15c3d11

…experimental/shared_cpu

Stats cleanup

4d83219

- Some stats were unused and broken(?) nuked them - added stats that are possible usefull to stdout (slightly ugly)

Merge branch 'master' of https://github.com/sdnfv/openNetVM-dev into …

98e6b39

…multithread Conflicts: examples/speed_tester/speed_tester.c

Add functionality to scale same NF instances

ab19825

Fixed a few global vars for multithreading to work

d756709

Code clean up, comments and style fixes

262a121

Added scale return value

a51b8ef

Fixed nf_mode for running advanced rings

5d93860

Refactored the onvm_nf/onvm_nf_info passing

0e0cc5b

- Keeping onvm_nf_info small, moved vars to onvm_nf - Added a callback function for the scaling child thread - Cleaned up code

Adv rings NF scaling, setup func for spawned NFS

5485492

- Adds ability to scale NFs with advanced rings - Adds a setup function for NFs - More docs and cleanup

Fixed build errors, changed NF typedef func names

ebf7f8e

Slightly working?

bdd7a28

Fixing bugs, cleaning up code, updating example NF

7539ca9

- Fixed the nf_info bug where the mempool would get messed up as children weren't cleared properly. - Updated the example scaling NF - Removed most of the debugging code

Clean up of shared cpu branch

ce693a8

Cleaned up messy bits, replacing macro branches with if statements, encapsulating code in functions, and getting rid of unnecessary code.

The code cleaning commit has arrived

96b9c90

- Style fixes - Removing debug code - Making code prettier - Adding better docs - Implementing requested changes

Minor cleanup/whitespace fixes

af20be3

koolzz added 2 commits May 30, 2019 22:32

Add sig handling global for adv rings

0020b0a

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

7bb14b3

…o_adv_rings Conflicts: examples/scaling_example/scaling.c examples/speed_tester/speed_tester.c onvm/onvm_nflib/onvm_common.h onvm/onvm_nflib/onvm_nflib.c onvm/onvm_nflib/onvm_nflib.h

onvm suggested changes May 31, 2019

View reviewed changes

Refactor, remove data pointer

acba319

koolzz changed the title ~~[WIP] Rewrite advanced rings API/examples~~ Rewrite advanced rings API/examples May 31, 2019

koolzz added 3 commits May 31, 2019 13:35

Cleanup, doc fixes

58004b1

Refactor scaling into 2 modes

57ebd5b

Cleanup some scaling code

aa09b44

onvm approved these changes May 31, 2019

View reviewed changes

kevindweb approved these changes Jun 1, 2019

View reviewed changes

onvm approved these changes Jun 2, 2019

View reviewed changes

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

13417d9

…o_adv_rings Conflicts: examples/scaling_example/scaling.c examples/speed_tester/speed_tester.c onvm/onvm_nflib/onvm_nflib.c

onvm approved these changes Jun 3, 2019

View reviewed changes

koolzz added 2 commits June 2, 2019 23:30

Sneaking in quick doc fix in here

1cf36af

Fix mgr termination

3f8f856

nks5295 approved these changes Jun 3, 2019

View reviewed changes

nks5295 changed the title ~~Rewrite advanced rings API/examples~~ Refactors Advanced Rings API/Examples Jun 3, 2019

nks5295 merged commit 7da175f into sdnfv:develop Jun 3, 2019

koolzz mentioned this pull request Jun 4, 2019

Advanced rings NF reconsiderations #120

Closed

koolzz deleted the redo_adv_rings branch June 7, 2019 23:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactors Advanced Rings API/Examples #124

Refactors Advanced Rings API/Examples #124

koolzz commented May 26, 2019 •

edited

Loading

koolzz commented May 31, 2019

onvm commented May 31, 2019

onvm left a comment

koolzz commented May 31, 2019

onvm commented May 31, 2019

onvm left a comment

koolzz commented May 31, 2019

kevindweb left a comment •

edited

Loading

koolzz commented Jun 2, 2019

onvm commented Jun 2, 2019

onvm left a comment

koolzz commented Jun 3, 2019

onvm commented Jun 3, 2019

onvm left a comment

koolzz commented Jun 3, 2019

Refactors Advanced Rings API/Examples #124

Refactors Advanced Rings API/Examples #124

Conversation

koolzz commented May 26, 2019 • edited Loading

Summary:

Merging notes:

Test Plan:

Review:

koolzz commented May 31, 2019

onvm commented May 31, 2019

CI Message

onvm left a comment

Choose a reason for hiding this comment

CI Message

Linter Output

koolzz commented May 31, 2019

onvm commented May 31, 2019

CI Message

onvm left a comment

Choose a reason for hiding this comment

CI Message

koolzz commented May 31, 2019

kevindweb left a comment • edited Loading

Choose a reason for hiding this comment

koolzz commented Jun 2, 2019

onvm commented Jun 2, 2019

CI Message

onvm left a comment

Choose a reason for hiding this comment

CI Message

koolzz commented Jun 3, 2019

onvm commented Jun 3, 2019

CI Message

onvm left a comment

Choose a reason for hiding this comment

CI Message

koolzz commented Jun 3, 2019

koolzz commented May 26, 2019 •

edited

Loading

kevindweb left a comment •

edited

Loading