Skip to content

Consolidates All NF Structs#117

Merged
nks5295 merged 183 commits intosdnfv:developfrom
koolzz:restructure_main_structs
May 31, 2019
Merged

Consolidates All NF Structs#117
nks5295 merged 183 commits intosdnfv:developfrom
koolzz:restructure_main_structs

Conversation

@koolzz
Copy link
Copy Markdown
Member

@koolzz koolzz commented May 16, 2019

This PR merges together onvm_nf_info and onvm_nf structs, into a single onvm_nf struct with internal variable groupings for readability, also renames key structs.

Summary:

  • The old onvm_nf_info is re purposed as onvm_nf_init_cfg a struct used to initialize the NF with the manager. This struct is passed to onvm_mgr and is then freed when initialization is complete.
  • In attempt to merge these structs the usage of onvm_nf_info has been removed from all internal and external API calls. Excluding the startup sequence.
  • The struct onvm_nf_context was renamed to onvm_nf_local_ctx
  • Added a nf_function_table struct that will hold all the NF defined callback functions.
  • Changed all NF facing API calls to pass onvm_nf_local_ctx instead of onvm_nf as NFs might need access to the keep_running variable.

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: None

TODO before merging :

  • PR is ready for review

Test Plan:

Regular testing + pktgen

Review:

Please review, release is near

sameergk and others added 30 commits February 13, 2017 13:26
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.
…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
 - 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
@koolzz
Copy link
Copy Markdown
Member Author

koolzz commented May 29, 2019

@onvm only speed tester works will you work

@onvm
Copy link
Copy Markdown

onvm commented May 29, 2019

@onvm only speed tester works will you work

CI Message

Your results will arrive shortly

@onvm
Copy link
Copy Markdown

onvm commented May 29, 2019

@onvm only speed tester works will you work

CI Message

Error: ERROR: Failed to fetch results from nimbnode30

Linter Output

onvm/onvm_nflib/onvm_nflib.c:465: Lines should be <= 120 characters long [whitespace/line_length] [5]
Total errors found: 1

@koolzz
Copy link
Copy Markdown
Member Author

koolzz commented May 29, 2019

I guess thats a no

Comment thread onvm/onvm_nflib/onvm_common.h
@koolzz
Copy link
Copy Markdown
Member Author

koolzz commented May 30, 2019

@onvm do your thing

@onvm
Copy link
Copy Markdown

onvm commented May 30, 2019

@onvm do your thing

CI Message

Your results will arrive shortly

@onvm
Copy link
Copy Markdown

onvm commented May 30, 2019

@onvm do your thing

CI Message

Error: ERROR: Failed to analyze results from nimbnode30

Linter Output

examples/aes_decrypt/aesdecrypt.c:173: Lines should be <= 120 characters long [whitespace/line_length] [5]
Total errors found: 1
examples/aes_encrypt/aesencrypt.c:173: Lines should be <= 120 characters long [whitespace/line_length] [5]
Total errors found: 1
examples/arp_response/arp_response.c:266: Lines should be <= 120 characters long [whitespace/line_length] [5]
Total errors found: 1
examples/basic_monitor/monitor.c:161: Lines should be <= 120 characters long [whitespace/line_length] [5]
Total errors found: 1
examples/bridge/bridge.c:142: Lines should be <= 120 characters long [whitespace/line_length] [5]
Total errors found: 1
examples/firewall/firewall.c:198: Lines should be <= 120 characters long [whitespace/line_length] [5]
Total errors found: 1
examples/flow_table/flow_table.c:208: Lines should be <= 120 characters long [whitespace/line_length] [5]
Total errors found: 1
examples/flow_tracker/flow_tracker.c:292: Lines should be <= 120 characters long [whitespace/line_length] [5]
Total errors found: 1
examples/load_balancer/load_balancer.c:488: Lines should be <= 120 characters long [whitespace/line_length] [5]
Total errors found: 1
examples/load_generator/load_generator.c:233: Lines should be <= 120 characters long [whitespace/line_length] [5]
Total errors found: 1
examples/nf_router/nf_router.c:212: Lines should be <= 120 characters long [whitespace/line_length] [5]
Total errors found: 1
examples/scaling_example/scaling.c:220: Lines should be <= 120 characters long [whitespace/line_length] [5]
Total errors found: 1
examples/simple_forward/forward.c:154: Lines should be <= 120 characters long [whitespace/line_length] [5]
Total errors found: 1
examples/speed_tester/speed_tester.c:279: Lines should be <= 120 characters long [whitespace/line_length] [5]
examples/speed_tester/speed_tester.c:517: Line ends in whitespace. Consider deleting these extra spaces. [whitespace/end_of_line] [4]
Total errors found: 2
examples/test_flow_dir/test_flow_dir.c:155: Lines should be <= 120 characters long [whitespace/line_length] [5]
Total errors found: 1
onvm/onvm_nflib/onvm_nflib.c:453: Lines should be <= 120 characters long [whitespace/line_length] [5]
onvm/onvm_nflib/onvm_nflib.c:466: Lines should be <= 120 characters long [whitespace/line_length] [5]
Total errors found: 2

@koolzz
Copy link
Copy Markdown
Member Author

koolzz commented May 30, 2019

@onvm you happy

@onvm
Copy link
Copy Markdown

onvm commented May 30, 2019

@onvm you happy

CI Message

Your results will arrive shortly

Copy link
Copy Markdown

@onvm onvm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@onvm you happy

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: 39169623
  • Performance rating - 111.91% (compared to 35000000 average)

Linter Output

onvm/onvm_nflib/onvm_nflib.c:467: Line ends in whitespace. Consider deleting these extra spaces. [whitespace/end_of_line] [4]
Total errors found: 1

@koolzz
Copy link
Copy Markdown
Member Author

koolzz commented May 30, 2019

@onvm lets do 3 out of 3

@onvm
Copy link
Copy Markdown

onvm commented May 30, 2019

@onvm lets do 3 out of 3

CI Message

Your results will arrive shortly

Copy link
Copy Markdown

@onvm onvm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@onvm lets do 3 out of 3

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: 39326084
  • Performance rating - 112.36% (compared to 35000000 average)

@nks5295 nks5295 changed the title Restructure main onvm_nf structs Consolidates All NF Structs May 31, 2019
@nks5295 nks5295 merged commit ac78a29 into sdnfv:develop May 31, 2019
koolzz added a commit to koolzz/openNetVM that referenced this pull request May 31, 2019
This PR merges together onvm_nf_info and onvm_nf structs, into a single onvm_nf struct with internal variable groupings for readability, also renames key structs.

Summary:

The old `onvm_nf_info` is re purposed as `onvm_nf_init_cfg` a struct used to initialize the NF with the manager. This struct is passed to onvm_mgr and is then freed when initialization is complete.
In attempt to merge these structs the usage of `onvm_nf_info` has been removed from all internal and external API calls. Excluding the startup sequence.
The struct `onvm_nf_context` was renamed to `onvm_nf_local_ctx`
Added a `nf_function_table` struct that will hold all the NF defined callback functions.
Changed all NF facing API calls to pass `onvm_nf_local_ctx` instead of onvm_nf as NFs might need access to the keep_running variable.

Commit log:

* Remove bootstrap struct from context

* Nuke some extra code

* Fix firewall API

* Remove unused globals in example NFs

* Fix style nit

* Fix double define

* Global rename/regrouping of onvm_nf struct

* Restructure scaling logic

* Rename nf_context->nf_local_ctx

* Further cleanup of scaling API

* Update method name

* Rename callbackhandlerfunc -> callback_func

* Fix linter

* Remove trailing whitespace

* Fix docs

* Fix more docs

* Free fix, code cleanup

* Set pointer to NULL on cleanup

* Testing function table struct

* Massive rename, API fixes, remove circular refr

* Don't mind me fixin whitespace

* Fix speed tester double func_table init bug

* Enforce 120 char limit

* Rem trailing whitespace

* Rem extra api call in speed_tester

* Fix docs

* Huuuuge linter fix, well tested
@koolzz koolzz deleted the restructure_main_structs branch June 7, 2019 23:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants