Shared core functionality for messages #149

dennisafa · 2019-06-25T17:24:34Z

Adds functionality to sleep when no messages are enqueued onto an nf's message ring.

Summary:

This is functionality I implemented as part of the larger mTCP project, in which an NF constantly receives lots of messages. This works exactly the same way as sleeping on an empty packet ring.

Usage:
Run the manager with shared CPU enabled.

This PR includes
Resolves issues
Breaking API changes
Internal API changes
Usability improvements
Bug fixes
New functionality	X
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:

Tested by sending a large influx of messages to an NF running in shared cpu mode. Also tested with multiple NF's on the same core. Checked htop CPU usage to verify proper sleeping when no messages were being sent.

Review:

Review checklist:

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

This PR releases OpenNetVM v19.05

This PR adds the following bug fix to the master branch: [Bug Fix] Fix Typo in Console Stats Header (sdnfv#142)

[Bug Fix] Fix Stats Header in Release Notes (sdnfv#145)

onvm · 2019-06-25T17:24:35Z

In response to PR creation

CI Message

Your results will arrive shortly

onvm · 2019-06-25T17:29:57Z

In response to PR creation

CI Message

Error: ERROR: Failed to fetch results from nimbnode30

…red_core_msg

dennisafa · 2019-06-27T03:00:33Z

@onvm try it now

kevindweb · 2019-06-28T02:51:08Z

@onvm can we pass this pktgen test?

onvm · 2019-06-28T02:51:10Z

@onvm can we pass this pktgen test?

CI Message

Your results will arrive shortly

onvm

@onvm can we pass this pktgen test?

CI Message

Run successful see results:
✔️ PR submitted to develop branch
✔️ Pktgen performance check passed
✔️ Speed Test performance check passed
✔️ Linter passed

[Results from nimbnode17]

Median TX pps for Pktgen: 6361442
Performance rating - 106.02% (compared to 6000000 average)
Median TX pps for Speed Tester: 38118038
Performance rating - 108.91% (compared to 35000000 average)

kevindweb · 2019-06-28T03:45:25Z

Yay to CI! I tested on cloudlab and got 13422053pps for pktgen. 4573120 rx/tx for 2 speed testers together. I'll test the specific message functionality later, but basic performance looks solid

koolzz

Overall I approve, see comment for Monday discussion

onvm/onvm_nflib/onvm_nflib.c

kevindweb · 2019-06-29T20:57:00Z

@onvm can we show dennis our new features?

onvm · 2019-06-29T20:57:01Z

@onvm can we show dennis our new features?

CI Message

Your results will arrive shortly

onvm

@onvm can we show dennis our new features?

CI Message

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

[Results from nimbnode17]

Median TX pps for Pktgen: 6313672
Performance rating - 63.14% (compared to 10000000 average)
Median TX pps for Speed Tester: 41629903
Performance rating - 111.01% (compared to 37500000 average)

kevindweb · 2019-06-29T21:08:42Z

@dennisafa pktgen from CI will work if you merge develop into this branch

koolzz

Good stuff, lets change this as we discussed in the meeting(only do the sleep check once one level up)

koolzz · 2019-07-08T19:49:32Z

@onvm perf?

kevindweb · 2019-07-08T20:10:46Z

@onvm perf

onvm · 2019-07-08T20:10:47Z

@onvm perf

CI Message

Your results will arrive shortly

onvm

@onvm perf

CI Message

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

[Results from nimbnode17]

Median TX pps for Pktgen: 0
Performance rating - 0.00% (compared to 10000000 average)
Median TX pps for Speed Tester: 40242712
Performance rating - 100.61% (compared to 40000000 average)

kevindweb · 2019-07-08T20:45:05Z

@onvm i believe in you

onvm · 2019-07-08T20:45:07Z

@onvm i believe in you

CI Message

Your results will arrive shortly

onvm

@onvm i believe in you

CI Message

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

[Results from nimbnode17]

Median TX pps for Pktgen: 6445082
Performance rating - 64.45% (compared to 10000000 average)
Median TX pps for Speed Tester: 40262852
Performance rating - 100.66% (compared to 40000000 average)

dennisafa · 2019-07-11T16:26:44Z

@onvm how's it goin pktgen

onvm · 2019-07-11T16:26:45Z

@onvm how's it goin pktgen

CI Message

Your results will arrive shortly

onvm

@onvm how's it goin pktgen

CI Message

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

[Results from nimbnode17]

Median TX pps for Pktgen: 0
Performance rating - 0.00% (compared to 10000000 average)
Median TX pps for Speed Tester: 40340438
Performance rating - 100.85% (compared to 40000000 average)

kevindweb · 2019-07-13T19:37:49Z

@onvm if you fail I'll catch you now!

onvm · 2019-07-13T19:37:50Z

@onvm if you fail I'll catch you now!

CI Message

Your results will arrive shortly

onvm

@onvm if you fail I'll catch you now!

CI Message

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

[Results from nimbnode17]

Median TX pps for Pktgen: 6354164
Performance rating - 63.54% (compared to 10000000 average)
Median TX pps for Speed Tester: 40907748
Performance rating - 102.27% (compared to 40000000 average)

koolzz

@dennisafa I approve just add a macro for msg threshold and should be good to merge

onvm/onvm_nflib/onvm_common.h

koolzz

Cool will merge soon, although please add a reamde note here, nf wakes up either when packets or a message arrives.

koolzz · 2019-07-24T20:11:58Z

@dennisafa just add the https://github.com/sdnfv/openNetVM/pull/151/files changes into this pr

dennisafa · 2019-07-25T12:56:18Z

@dennisafa just add the https://github.com/sdnfv/openNetVM/pull/151/files changes into this pr

oops. got it.

onvm · 2019-07-25T22:12:11Z

Testing

CI Message

Your results will arrive shortly

onvm · 2019-07-25T22:18:20Z

Testing

CI Message

Error: Failed to parse Speed Tester stats

onvm · 2019-07-25T22:26:16Z

Testing

CI Message

Your results will arrive shortly

onvm

Testing

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 Pktgen: 6408064
Performance rating - 64.08% (compared to 10000000 average)
Median TX pps for Speed Tester: 40242298
Performance rating - 100.61% (compared to 40000000 average)
Median TX pps for mtcp: 0.236000
Performance rating - 102.61% (compared to 0.230000 average)

kevindweb

Let's goooooooo! First successful mTCP CI run. I also have been testing on cloudlab, when I merged develop, and disabled flow table I got 14746136pps Pktgen, and 12895796pps with ft macro enabled, which is normal. Created a 4 speed tester chain with performance of ~17664647 pps tx on all 4, and got almost 52mil pps just running one speed_tester. Also, not a change request, just a question, was there a performance benefit to moving this code out of onvm_nflib_dequeue_packets, or did it just make more logical sense? I ask this because there doesn't seem to be a whole lot changed, except for the condition definition.

dennisafa · 2019-07-26T00:29:32Z

awesome, im excited CI has mTCP results now. no real performance benefit, just the fact that the semwait call is logically equivalent so having it in both dequeue_messages and dequeue_packets would be extra

…

On Thu, Jul 25, 2019 at 6:45 PM Kevin Deems ***@***.***> wrote: ***@***.**** approved this pull request. Let's goooooooo! First successful mTCP CI run. I also have been testing on cloudlab, when I merged develop, and disabled flow table I got 14746136pps Pktgen, and 12895796pps with ft macro enabled, which is normal. Created a 4 speed tester chain with performance of ~17664647 pps tx on all 4, and got almost 52mil pps just running one speed_tester. Also, not a change request, just a question, was there a performance benefit to moving this code out of onvm_nflib_dequeue_packets, or did it just make more logical sense? I ask this because there doesn't seem to be a whole lot changed, except for the condition definition. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#149?email_source=notifications&email_token=AH3EIZW6GDMOG5PBQUKBPTTQBIUJ7A5CNFSM4H3KRXKKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOB7UOMZY#pullrequestreview-266921575>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AH3EIZTUNKSZLJL325TGXLTQBIUJ7ANCNFSM4H3KRXKA> .

koolzz

Noice

koolzz and others added 4 commits June 4, 2019 14:45

Merge pull request sdnfv#134 from sdnfv/develop

46bbc96

This PR releases OpenNetVM v19.05

Merge pull request sdnfv#144 from sdnfv/develop

ea0bf0c

This PR adds the following bug fix to the master branch: [Bug Fix] Fix Typo in Console Stats Header (sdnfv#142)

Merge pull request sdnfv#146 from sdnfv/develop

c12d26d

[Bug Fix] Fix Stats Header in Release Notes (sdnfv#145)

Shared cores for messages

ed5295b

dennisafa assigned koolzz and kevindweb Jun 25, 2019

dennisafa added the enhancement 🚀 label Jun 25, 2019

dennisafa unassigned koolzz and kevindweb Jun 25, 2019

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

c00f6a2

…red_core_msg

dennisafa changed the base branch from master to develop June 27, 2019 03:00

root added 2 commits June 26, 2019 22:03

Rm line

e700b57

nf context, not nf

e34497a

onvm approved these changes Jun 28, 2019

View reviewed changes

koolzz reviewed Jun 28, 2019

View reviewed changes

onvm/onvm_nflib/onvm_nflib.c Outdated Show resolved Hide resolved

onvm/onvm_nflib/onvm_nflib.c Outdated Show resolved Hide resolved

onvm suggested changes Jun 29, 2019

View reviewed changes

koolzz requested changes Jul 3, 2019

View reviewed changes

Moved functionality to main thread

be13c4a

onvm suggested changes Jul 8, 2019

View reviewed changes

onvm suggested changes Jul 11, 2019

View reviewed changes

onvm suggested changes Jul 13, 2019

View reviewed changes

koolzz previously approved these changes Jul 14, 2019

View reviewed changes

onvm/onvm_nflib/onvm_common.h Outdated Show resolved Hide resolved

Adding macro for msg threshold

9d435db

dennisafa dismissed koolzz’s stale review via 9d435db July 19, 2019 15:44

koolzz previously approved these changes Jul 20, 2019

View reviewed changes

koolzz mentioned this pull request Jul 24, 2019

Update NF_DEV.md #151

Closed

dennisafa requested a review from kevindweb July 25, 2019 13:01

Update NF_Dev.md

cf8aa2a

dennisafa dismissed koolzz’s stale review via cf8aa2a July 25, 2019 13:13

onvm suggested changes Jul 25, 2019

View reviewed changes

kevindweb approved these changes Jul 25, 2019

View reviewed changes

koolzz approved these changes Jul 27, 2019

View reviewed changes

koolzz merged commit 60293fa into sdnfv:develop Jul 27, 2019

dennisafa deleted the shared_core_msg branch July 28, 2019 19:13

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

Shared core functionality for messages #149

Shared core functionality for messages #149

Conversation

dennisafa commented Jun 25, 2019

Summary:

Merging notes:

Test Plan:

Review:

Review checklist:

onvm commented Jun 25, 2019

CI Message

onvm commented Jun 25, 2019

CI Message

dennisafa commented Jun 27, 2019

kevindweb commented Jun 28, 2019

onvm commented Jun 28, 2019

CI Message

onvm left a comment

Choose a reason for hiding this comment

CI Message

kevindweb commented Jun 28, 2019

koolzz left a comment

Choose a reason for hiding this comment

kevindweb commented Jun 29, 2019

onvm commented Jun 29, 2019

CI Message

onvm left a comment

Choose a reason for hiding this comment

CI Message

kevindweb commented Jun 29, 2019

koolzz left a comment

Choose a reason for hiding this comment

koolzz commented Jul 8, 2019

kevindweb commented Jul 8, 2019

onvm commented Jul 8, 2019

CI Message

onvm left a comment

Choose a reason for hiding this comment

CI Message

kevindweb commented Jul 8, 2019

onvm commented Jul 8, 2019

CI Message

onvm left a comment

Choose a reason for hiding this comment

CI Message

dennisafa commented Jul 11, 2019

onvm commented Jul 11, 2019

CI Message

onvm left a comment

Choose a reason for hiding this comment

CI Message

kevindweb commented Jul 13, 2019

onvm commented Jul 13, 2019

CI Message

onvm left a comment

Choose a reason for hiding this comment

CI Message

koolzz left a comment

Choose a reason for hiding this comment

koolzz left a comment

Choose a reason for hiding this comment

koolzz commented Jul 24, 2019

dennisafa commented Jul 25, 2019

onvm commented Jul 25, 2019

CI Message

onvm commented Jul 25, 2019

CI Message

onvm commented Jul 25, 2019

CI Message

onvm left a comment

Choose a reason for hiding this comment

CI Message

kevindweb left a comment

Choose a reason for hiding this comment

dennisafa commented Jul 26, 2019 via email

koolzz left a comment

Choose a reason for hiding this comment