Skip to content

Upgrading from Ubuntu 14.04.1 to 18.04.1#72

Merged
koolzz merged 39 commits intosdnfv:developfrom
dennisafa:ubuntu18
Mar 21, 2019
Merged

Upgrading from Ubuntu 14.04.1 to 18.04.1#72
koolzz merged 39 commits intosdnfv:developfrom
dennisafa:ubuntu18

Conversation

@dennisafa
Copy link
Copy Markdown
Member

@dennisafa dennisafa commented Mar 4, 2019

Modifies necessary files to make ONVM compatible with Ubuntu 18.04.1 LTS

Summary:

With the new Ubuntu version comes an upgraded version of gcc, which has stricter checks in place with regards to strict aliasing, formatting, etc. This pull request modifies code where these errors occured.

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 pktgen
  • Test mTCP

Test Plan:

Testing was done on a bare-metal cloudlab node running 18.04.1. Speed tester was run with the following results:

Speed tester
1 NF 52 million pkt/s
2 NF 18 million pkt/s

Update: testing with pktgen resulted in a peak of 7 million pps.

Review:

  • Sanity checks, assigned to @koolzz

    • Run make in /onvm and /examples directories
    • Test new functionality or bug fix from the pr (if needed)
  • Code style, assigned to @koolzz

    • Run linter
    • Check everything style related
  • Code design, assigned to @koolzz

    • 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

    • Run Speed Tester NF, report performance.
    • Run pktgen, report performance (if needed)
    • Run mTCP epserver, verify wget works, report performance for epwget/ab (if needed)
  • Documentation, assigned to @koolzz

    • Check if the new changes are well documented, in both code and READMEs

@koolzz koolzz self-requested a review March 4, 2019 19:49
Comment thread onvm/lib/cJSON.c Outdated
/* 10xxxxxx */
*--ptr2 = ((uc | 0x80) & 0xBF);
uc >>= 6;
break;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This technically changes the behavior, instead of falling through all the cases it only does the one.

Although I'm not sure what the expected behavior is anyway. Should we just get a new cJSON version?

Copy link
Copy Markdown
Member Author

@dennisafa dennisafa Mar 6, 2019

Choose a reason for hiding this comment

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

edit: added comments to let the statement fallthrough without triggering the compiler warning. i will look into new cJSON versions.

@sdnfv sdnfv deleted a comment from onvm Mar 9, 2019
@sdnfv sdnfv deleted a comment from onvm Mar 9, 2019
@sdnfv sdnfv deleted a comment from onvm Mar 9, 2019
@sdnfv sdnfv deleted a comment from onvm Mar 9, 2019
@koolzz
Copy link
Copy Markdown
Member

koolzz commented Mar 9, 2019

@onvm whats so different about the public repo

@onvm
Copy link
Copy Markdown

onvm commented Mar 9, 2019

openNetVM

CI Message

Your results will arrive shortly

@onvm
Copy link
Copy Markdown

onvm commented Mar 9, 2019

openNetVM

CI Message

Error: ERROR: Failed to fetch and checkout pull request

Linter Passed

@koolzz
Copy link
Copy Markdown
Member

koolzz commented Mar 9, 2019

@onvm public repo = no pexpect pwd. @AaronCoplan 👍

@onvm

This comment has been minimized.

@onvm

This comment has been minimized.

@koolzz
Copy link
Copy Markdown
Member

koolzz commented Mar 9, 2019

@onvm one more time with the right response message?

@onvm
Copy link
Copy Markdown

onvm commented Mar 9, 2019

@onvm one more time with the right response message?

CI Message

Your results will arrive shortly

@koolzz
Copy link
Copy Markdown
Member

koolzz commented Mar 9, 2019

@onvm show people what happens when you're busy working

@onvm
Copy link
Copy Markdown

onvm commented Mar 9, 2019

@onvm show people what happens when you're busy working

CI Message

Another CI run in progress, please try again in 15 minutes

@onvm
Copy link
Copy Markdown

onvm commented Mar 9, 2019

@onvm one more time with the right response message?

CI Message

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

Linter Failed

examples/aes_decrypt/aes.c:523: If an else has a brace on one side, it should have it on both [readability/braces] [5]
examples/aes_decrypt/aes.c:1024: If an else has a brace on one side, it should have it on both [readability/braces] [5]
examples/aes_decrypt/aes.c:1028: If an else has a brace on one side, it should have it on both [readability/braces] [5]
examples/aes_decrypt/aes.c:1081: If an else has a brace on one side, it should have it on both [readability/braces] [5]
examples/aes_decrypt/aes.c:1087: If an else has a brace on one side, it should have it on both [readability/braces] [5]
Total errors found: 5
examples/aes_encrypt/aes.c:523: If an else has a brace on one side, it should have it on both [readability/braces] [5]
examples/aes_encrypt/aes.c:1024: If an else has a brace on one side, it should have it on both [readability/braces] [5]
examples/aes_encrypt/aes.c:1028: If an else has a brace on one side, it should have it on both [readability/braces] [5]
examples/aes_encrypt/aes.c:1081: If an else has a brace on one side, it should have it on both [readability/braces] [5]
examples/aes_encrypt/aes.c:1087: If an else has a brace on one side, it should have it on both [readability/braces] [5]
Total errors found: 5
examples/arp_response/arp_response.c:144: Missing space before ( in switch( [whitespace/parens] [5]
Total errors found: 1
examples/bridge/bridge.c:135: Missing space before ( in if( [whitespace/parens] [5]
examples/bridge/bridge.c:138: If an else has a brace on one side, it should have it on both [readability/braces] [5]
examples/bridge/bridge.c:156: If an else has a brace on one side, it should have it on both [readability/braces] [5]
Total errors found: 3
examples/flow_table/flow_table.c:103: Missing space before ( in if( [whitespace/parens] [5]
examples/flow_table/flow_table.c:198: Missing space before ( in if( [whitespace/parens] [5]
examples/flow_table/flow_table.c:216: Missing space before ( in if( [whitespace/parens] [5]
examples/flow_table/flow_table.c:224: Missing space before ( in if( [whitespace/parens] [5]
examples/flow_table/flow_table.c:231: If an else has a brace on one side, it should have it on both [readability/braces] [5]
examples/flow_table/flow_table.c:238: If an else has a brace on one side, it should have it on both [readability/braces] [5]
Total errors found: 6
examples/flow_table/msgbuf.c:71: Missing space before ( in if( [whitespace/parens] [5]
examples/flow_table/msgbuf.c:73: Missing space before ( in if( [whitespace/parens] [5]
examples/flow_table/msgbuf.c:83: Missing space before ( in while( [whitespace/parens] [5]
examples/flow_table/msgbuf.c:86: Missing space before ( in if( [whitespace/parens] [5]
examples/flow_table/msgbuf.c:89: Missing space before ( in if( [whitespace/parens] [5]
examples/flow_table/msgbuf.c:106: Missing space before ( in if( [whitespace/parens] [5]
examples/flow_table/msgbuf.c:108: Missing space before ( in if( [whitespace/parens] [5]
examples/flow_table/msgbuf.c:116: Missing space before ( in while( [whitespace/parens] [5]
examples/flow_table/msgbuf.c:119: Missing space before ( in if( [whitespace/parens] [5]
examples/flow_table/msgbuf.c:125: Missing space before ( in if( [whitespace/parens] [5]
examples/flow_table/msgbuf.c:138: Extra space before last semicolon. If this should be an empty statement, use {} instead. [whitespace/semicolon] [5]
examples/flow_table/msgbuf.c:140: Missing space before ( in if( [whitespace/parens] [5]
examples/flow_table/msgbuf.c:149: Missing space before ( in if( [whitespace/parens] [5]
examples/flow_table/msgbuf.c:157: Missing space before ( in if( [whitespace/parens] [5]
examples/flow_table/msgbuf.c:159: Missing space before ( in if( [whitespace/parens] [5]
examples/flow_table/msgbuf.c:162: Missing space before ( in if( [whitespace/parens] [5]
examples/flow_table/msgbuf.c:169: Missing space before ( in while( [whitespace/parens] [5]
Total errors found: 17
examples/flow_table/sdn.c:99: Extra space before last semicolon. If this should be an empty statement, use {} instead. [whitespace/semicolon] [5]
examples/flow_table/sdn.c:125: Missing space before ( in if( [whitespace/parens] [5]
examples/flow_table/sdn.c:127: Missing space before ( in if( [whitespace/parens] [5]
examples/flow_table/sdn.c:147: If an else has a brace on one side, it should have it on both [readability/braces] [5]
examples/flow_table/sdn.c:147: Missing space before { [whitespace/braces] [5]
examples/flow_table/sdn.c:153: Missing space before ( in while( [whitespace/parens] [5]
examples/flow_table/sdn.c:160: Missing space before { [whitespace/braces] [5]
examples/flow_table/sdn.c:235: If an else has a brace on one side, it should have it on both [readability/braces] [5]
examples/flow_table/sdn.c:239: If an else has a brace on one side, it should have it on both [readability/braces] [5]
examples/flow_table/sdn.c:279: Missing space before ( in if( [whitespace/parens] [5]
examples/flow_table/sdn.c:418: Missing space before ( in if( [whitespace/parens] [5]
examples/flow_table/sdn.c:423: Missing space before ( in if( [whitespace/parens] [5]
examples/flow_table/sdn.c:493: If an else has a brace on one side, it should have it on both [readability/braces] [5]
examples/flow_table/sdn.c:501: If an else has a brace on one side, it should have it on both [readability/braces] [5]
examples/flow_table/sdn.c:538: Missing space before ( in if( [whitespace/parens] [5]
examples/flow_table/sdn.c:542: Missing space before ( in if( [whitespace/parens] [5]
examples/flow_table/sdn.c:545: Missing space before ( in if( [whitespace/parens] [5]
examples/flow_table/sdn.c:547: Missing space before ( in if( [whitespace/parens] [5]
examples/flow_table/sdn.c:555: Missing space before ( in if( [whitespace/parens] [5]
examples/flow_table/sdn.c:569: Missing space before ( in while( [whitespace/parens] [5]
Total errors found: 20
examples/flow_table/setupconn.c:82: Missing space before ( in if( [whitespace/parens] [5]
examples/flow_table/setupconn.c:89: Missing space before ( in if( [whitespace/parens] [5]
examples/flow_table/setupconn.c:94: Missing space before ( in if( [whitespace/parens] [5]
examples/flow_table/setupconn.c:102: Missing space before ( in if( [whitespace/parens] [5]
examples/flow_table/setupconn.c:110: Missing space before { [whitespace/braces] [5]
examples/flow_table/setupconn.c:112: Missing space before { [whitespace/braces] [5]
examples/flow_table/setupconn.c:122: Missing space before ( in if( [whitespace/parens] [5]
examples/flow_table/setupconn.c:140: Missing space before ( in if( [whitespace/parens] [5]
examples/flow_table/setupconn.c:149: Missing space before ( in if( [whitespace/parens] [5]
examples/flow_table/setupconn.c:156: Missing space before ( in if( [whitespace/parens] [5]
Total errors found: 10
examples/flow_tracker/flow_tracker.c:108: Missing space before ( in switch( [whitespace/parens] [5]
examples/flow_tracker/flow_tracker.c:210: If an else has a brace on one side, it should have it on both [readability/braces] [5]
Total errors found: 2
examples/scaling_example/scaling.c:312: Missing space before ( in while( [whitespace/parens] [5]
Total errors found: 1
onvm/onvm_mgr/main.c:98: Mismatching spaces inside () in while [whitespace/parens] [5]
Total errors found: 1
onvm/onvm_mgr/onvm_args.c:166: Missing space before ( in if( [whitespace/parens] [5]
onvm/onvm_mgr/onvm_args.c:166: Missing space before { [whitespace/braces] [5]
onvm/onvm_mgr/onvm_args.c:172: Missing space before ( in if( [whitespace/parens] [5]
onvm/onvm_mgr/onvm_args.c:172: Missing space before { [whitespace/braces] [5]
onvm/onvm_mgr/onvm_args.c:328: Missing space before ( in if( [whitespace/parens] [5]
onvm/onvm_mgr/onvm_args.c:352: Missing space before { [whitespace/braces] [5]
onvm/onvm_mgr/onvm_args.c:357: Missing space before ( in if( [whitespace/parens] [5]
Total errors found: 7
onvm/onvm_mgr/onvm_init.c:222: Never use sprintf. Use snprintf instead. [runtime/printf] [5]
Total errors found: 1
onvm/onvm_mgr/onvm_nf.c:182: Missing space before ( in if( [whitespace/parens] [5]
onvm/onvm_mgr/onvm_nf.c:255: Missing space before ( in if( [whitespace/parens] [5]
Total errors found: 2
onvm/onvm_mgr/onvm_stats.c:183: Missing space before ( in if( [whitespace/parens] [5]
onvm/onvm_mgr/onvm_stats.c:183: Missing space before { [whitespace/braces] [5]
Total errors found: 2
onvm/onvm_nflib/onvm_config_common.c:52: Missing space before { [whitespace/braces] [5]
onvm/onvm_nflib/onvm_config_common.c:112: Missing space before ( in for( [whitespace/parens] [5]
onvm/onvm_nflib/onvm_config_common.c:112: Missing space before { [whitespace/braces] [5]
onvm/onvm_nflib/onvm_config_common.c:417: Never use sprintf. Use snprintf instead. [runtime/printf] [5]
onvm/onvm_nflib/onvm_config_common.c:433: Never use sprintf. Use snprintf instead. [runtime/printf] [5]
onvm/onvm_nflib/onvm_config_common.c:487: Never use sprintf. Use snprintf instead. [runtime/printf] [5]
Total errors found: 6
onvm/onvm_nflib/onvm_flow_dir.c:64: Missing space before ( in if( [whitespace/parens] [5]
onvm/onvm_nflib/onvm_flow_dir.c:95: Missing space before { [whitespace/braces] [5]
onvm/onvm_nflib/onvm_flow_dir.c:103: Missing space before { [whitespace/braces] [5]
onvm/onvm_nflib/onvm_flow_dir.c:111: Missing space before { [whitespace/braces] [5]
onvm/onvm_nflib/onvm_flow_dir.c:128: Missing space before { [whitespace/braces] [5]
onvm/onvm_nflib/onvm_flow_dir.c:143: Missing space before { [whitespace/braces] [5]
onvm/onvm_nflib/onvm_flow_dir.c:151: Missing space before { [whitespace/braces] [5]
onvm/onvm_nflib/onvm_flow_dir.c:159: Missing space before { [whitespace/braces] [5]
onvm/onvm_nflib/onvm_flow_dir.c:176: Missing space before { [whitespace/braces] [5]
Total errors found: 9
onvm/onvm_nflib/onvm_nflib.c:341: Missing space before ( in if( [whitespace/parens] [5]
onvm/onvm_nflib/onvm_nflib.c:344: Missing space before ( in if( [whitespace/parens] [5]
onvm/onvm_nflib/onvm_nflib.c:347: Missing space before ( in if( [whitespace/parens] [5]
onvm/onvm_nflib/onvm_nflib.c:351: Missing space before ( in if( [whitespace/parens] [5]
onvm/onvm_nflib/onvm_nflib.c:354: Missing space before ( in if( [whitespace/parens] [5]
onvm/onvm_nflib/onvm_nflib.c:357: Missing space before ( in if( [whitespace/parens] [5]
onvm/onvm_nflib/onvm_nflib.c:584: Missing space before ( in switch( [whitespace/parens] [5]
onvm/onvm_nflib/onvm_nflib.c:743: Missing space before ( in if( [whitespace/parens] [5]
onvm/onvm_nflib/onvm_nflib.c:754: Missing space before ( in if( [whitespace/parens] [5]
onvm/onvm_nflib/onvm_nflib.c:821: Missing space before { [whitespace/braces] [5]
Total errors found: 10
onvm/onvm_nflib/onvm_pkt_helper.c:313: Missing space before ( in switch( [whitespace/parens] [5]
onvm/onvm_nflib/onvm_pkt_helper.c:376: Missing space before { [whitespace/braces] [5]
Total errors found: 2

Copy link
Copy Markdown
Member

@koolzz koolzz left a comment

Choose a reason for hiding this comment

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

Tested on cloudlab, builds fine with these changes

@koolzz
Copy link
Copy Markdown
Member

koolzz commented Mar 13, 2019

@dennisafa were you able to get mTCP running with this?

@dennisafa
Copy link
Copy Markdown
Member Author

@dennisafa were you able to get mTCP running with this?

i haven't had a chance to try out mTCP quite yet. i'll get that done by the end of the day.

@dennisafa dennisafa mentioned this pull request Mar 18, 2019
4 tasks
@koolzz
Copy link
Copy Markdown
Member

koolzz commented Mar 21, 2019

As discussed in the meeting, mTCP might have some code not compatible with ubuntu 18.04.1. This pr should still be good to merge as long as Pktgen & onvm works. Will setup the ubuntu 18.04.1 cloudlab experiment and merge if everything functions as expected.

@dennisafa
Copy link
Copy Markdown
Member Author

As discussed in the meeting, mTCP might have some code not compatible with ubuntu 18.04.1. This pr should still be good to merge as long as Pktgen & onvm works. Will setup the ubuntu 18.04.1 cloudlab experiment and merge if everything functions as expected.

mTCP compatibility could be made into a separate PR.

@koolzz
Copy link
Copy Markdown
Member

koolzz commented Mar 21, 2019

Yeah, and it would be in the mtcp repo anyway.

@koolzz koolzz merged commit f8488cc into sdnfv:develop Mar 21, 2019
@dennisafa dennisafa deleted the ubuntu18 branch April 27, 2019 02:53
@koolzz koolzz added this to the ONVM 19.05 Release milestone May 26, 2019
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.

3 participants