Skip to content

Fix NF Tag memory allocation#103

Merged
koolzz merged 3 commits intosdnfv:developfrom
koolzz:nf_tag_fix
Apr 21, 2019
Merged

Fix NF Tag memory allocation#103
koolzz merged 3 commits intosdnfv:developfrom
koolzz:nf_tag_fix

Conversation

@koolzz
Copy link
Copy Markdown
Member

@koolzz koolzz commented Apr 17, 2019

Fix NF Tag to properly display it in stats later

Summary:

Before we were passing a const char * for the nf tag and the pointer was local NF stack memory, which was clearly not accessible by the onvm_mgr. This pr fixes this issue by placing nf_tag on the heap and defining a max length of the provided nf tag.

This has no breaking changes as its all internal logic.

With this PR we have everything to rework stdout stats with both a tag and the core info(lets discuss formatting details in the meeting). Also, @kevindweb you gotta handle webstats, every NF can now be properly labeled (Speed Tester 1) instead of (NF 1) 😉

Usage:
Tested by printing %s with tag from the onvm_mgr stats output.

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 :

  • Might want to update all the current tags?
  • PR is ready for review

Test Plan:

Test functionality, makes sure onvm no break

Review:

TBA

@onvm
Copy link
Copy Markdown

onvm commented Apr 17, 2019

In response to PR creation

CI Message

Your results will arrive shortly

@onvm
Copy link
Copy Markdown

onvm commented Apr 17, 2019

In response to PR creation

CI Message

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

Linter Failed

examples/aes_decrypt/aes.h:176: #endif line should be "#endif // AES_H" [build/header_guard] [5]
Total errors found: 1
examples/aes_encrypt/aes.h:185: #endif line should be "#endif // AES_H" [build/header_guard] [5]
Total errors found: 1
examples/flow_table/flow_table.h:63: #endif line should be "#endif // FLOW_TABLE_H" [build/header_guard] [5]
Total errors found: 1
examples/flow_table/msgbuf.h:71: #endif line should be "#endif // MSGBUF_H" [build/header_guard] [5]
Total errors found: 1
examples/flow_table/openflow.h:969: #endif line should be "#endif // OPENFLOW_H" [build/header_guard] [5]
examples/flow_table/openflow.h:50: Using deprecated casting style. Use static_cast(...) instead [readability/casting] [4]
examples/flow_table/openflow.h:569: Extra space before ( in function call [whitespace/parens] [4]
examples/flow_table/openflow.h:634: Extra space before ( in function call [whitespace/parens] [4]
examples/flow_table/openflow.h:771: Extra space before ( in function call [whitespace/parens] [4]
examples/flow_table/openflow.h:804: Extra space before ( in function call [whitespace/parens] [4]
examples/flow_table/openflow.h:865: Extra space before ( in function call [whitespace/parens] [4]
examples/flow_table/openflow.h:926: Extra space before ( in function call [whitespace/parens] [4]
Total errors found: 8
examples/flow_table/sdn.c:334: { should almost always be at the end of the previous line [whitespace/braces] [4]
examples/flow_table/sdn.c:365: Should have a space between // and comment [whitespace/comments] [4]
Total errors found: 2
examples/flow_table/sdn.h:104: #endif line should be "#endif // SDN_H" [build/header_guard] [5]
Total errors found: 1
examples/flow_table/sdn_pkt_list.h:122: #endif line should be "#endif // SDN_PKT_LIST_H" [build/header_guard] [5]
Total errors found: 1
examples/flow_table/setupconn.h:53: #endif line should be "#endif // SETUPCONN_H" [build/header_guard] [5]
Total errors found: 1
onvm/onvm_mgr/onvm_init.c:107: { should almost always be at the end of the previous line [whitespace/braces] [4]
onvm/onvm_mgr/onvm_init.c:114: { should almost always be at the end of the previous line [whitespace/braces] [4]
onvm/onvm_mgr/onvm_init.c:116: { should almost always be at the end of the previous line [whitespace/braces] [4]
Total errors found: 3
onvm/onvm_mgr/onvm_pkt.c:68: Are you taking an address of a cast? This is dangerous: could be a temp var. Take the address before doing the cast, rather than after [runtime/casting] [4]
Total errors found: 1
onvm/onvm_nflib/onvm_common.h:367: #endif line should be "#endif // ONVM_COMMON_H" [build/header_guard] [5]
Total errors found: 1
onvm/onvm_nflib/onvm_config_common.h:208: #endif line should be "#endif // ONVM_CONFIG_COMMON_H" [build/header_guard] [5]
Total errors found: 1
onvm/onvm_nflib/onvm_msg_common.h:61: #endif line should be "#endif // ONVM_MSG_COMMON_H" [build/header_guard] [5]
Total errors found: 1
onvm/onvm_nflib/onvm_nflib.c:523: Line ends in whitespace. Consider deleting these extra spaces. [whitespace/end_of_line] [4]
Total errors found: 1
onvm/onvm_nflib/onvm_pkt_common.c:98: Are you taking an address of a cast? This is dangerous: could be a temp var. Take the address before doing the cast, rather than after [runtime/casting] [4]
Total errors found: 1
onvm/onvm_nflib/onvm_sc_common.h:70: #endif line should be "#endif // ONVM_SC_COMMON_H" [build/header_guard] [5]
onvm/onvm_nflib/onvm_sc_common.h:70: Should have a space between // and comment [whitespace/comments] [4]
Total errors found: 2
onvm/onvm_nflib/onvm_sc_mgr.h:79: #endif line should be "#endif // ONVM_SC_MGR_H" [build/header_guard] [5]
Total errors found: 1

@koolzz
Copy link
Copy Markdown
Member Author

koolzz commented Apr 20, 2019

@kevindweb you have used this for webstuff so give it a review

Copy link
Copy Markdown
Contributor

@kevindweb kevindweb left a comment

Choose a reason for hiding this comment

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

Works perfectly, I have been using it for visualization in the web stats app.

@koolzz
Copy link
Copy Markdown
Member Author

koolzz commented Apr 21, 2019

Just in case @onvm

@onvm
Copy link
Copy Markdown

onvm commented Apr 21, 2019

Just in case @onvm

CI Message

Your results will arrive shortly

@onvm
Copy link
Copy Markdown

onvm commented Apr 21, 2019

Just in case @onvm

CI Message

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

Linter Failed

examples/aes_decrypt/aes.h:176: #endif line should be "#endif // AES_H" [build/header_guard] [5]
Total errors found: 1
examples/aes_encrypt/aes.h:185: #endif line should be "#endif // AES_H" [build/header_guard] [5]
Total errors found: 1
examples/flow_table/flow_table.h:63: #endif line should be "#endif // FLOW_TABLE_H" [build/header_guard] [5]
Total errors found: 1
examples/flow_table/msgbuf.h:71: #endif line should be "#endif // MSGBUF_H" [build/header_guard] [5]
Total errors found: 1
examples/flow_table/openflow.h:969: #endif line should be "#endif // OPENFLOW_H" [build/header_guard] [5]
examples/flow_table/openflow.h:50: Using deprecated casting style. Use static_cast(...) instead [readability/casting] [4]
examples/flow_table/openflow.h:569: Extra space before ( in function call [whitespace/parens] [4]
examples/flow_table/openflow.h:634: Extra space before ( in function call [whitespace/parens] [4]
examples/flow_table/openflow.h:771: Extra space before ( in function call [whitespace/parens] [4]
examples/flow_table/openflow.h:804: Extra space before ( in function call [whitespace/parens] [4]
examples/flow_table/openflow.h:865: Extra space before ( in function call [whitespace/parens] [4]
examples/flow_table/openflow.h:926: Extra space before ( in function call [whitespace/parens] [4]
Total errors found: 8
examples/flow_table/sdn.c:334: { should almost always be at the end of the previous line [whitespace/braces] [4]
examples/flow_table/sdn.c:365: Should have a space between // and comment [whitespace/comments] [4]
Total errors found: 2
examples/flow_table/sdn.h:104: #endif line should be "#endif // SDN_H" [build/header_guard] [5]
Total errors found: 1
examples/flow_table/sdn_pkt_list.h:122: #endif line should be "#endif // SDN_PKT_LIST_H" [build/header_guard] [5]
Total errors found: 1
examples/flow_table/setupconn.h:53: #endif line should be "#endif // SETUPCONN_H" [build/header_guard] [5]
Total errors found: 1
onvm/onvm_mgr/onvm_init.c:107: { should almost always be at the end of the previous line [whitespace/braces] [4]
onvm/onvm_mgr/onvm_init.c:114: { should almost always be at the end of the previous line [whitespace/braces] [4]
onvm/onvm_mgr/onvm_init.c:116: { should almost always be at the end of the previous line [whitespace/braces] [4]
Total errors found: 3
onvm/onvm_mgr/onvm_pkt.c:68: Are you taking an address of a cast? This is dangerous: could be a temp var. Take the address before doing the cast, rather than after [runtime/casting] [4]
Total errors found: 1
onvm/onvm_nflib/onvm_common.h:367: #endif line should be "#endif // ONVM_COMMON_H" [build/header_guard] [5]
Total errors found: 1
onvm/onvm_nflib/onvm_config_common.h:208: #endif line should be "#endif // ONVM_CONFIG_COMMON_H" [build/header_guard] [5]
Total errors found: 1
onvm/onvm_nflib/onvm_msg_common.h:61: #endif line should be "#endif // ONVM_MSG_COMMON_H" [build/header_guard] [5]
Total errors found: 1
onvm/onvm_nflib/onvm_nflib.c:523: Line ends in whitespace. Consider deleting these extra spaces. [whitespace/end_of_line] [4]
Total errors found: 1
onvm/onvm_nflib/onvm_pkt_common.c:98: Are you taking an address of a cast? This is dangerous: could be a temp var. Take the address before doing the cast, rather than after [runtime/casting] [4]
Total errors found: 1
onvm/onvm_nflib/onvm_sc_common.h:70: #endif line should be "#endif // ONVM_SC_COMMON_H" [build/header_guard] [5]
onvm/onvm_nflib/onvm_sc_common.h:70: Should have a space between // and comment [whitespace/comments] [4]
Total errors found: 2
onvm/onvm_nflib/onvm_sc_mgr.h:79: #endif line should be "#endif // ONVM_SC_MGR_H" [build/header_guard] [5]
Total errors found: 1

@koolzz koolzz merged commit b9611f7 into sdnfv:develop Apr 21, 2019
@koolzz koolzz added this to the ONVM 19.05 Release milestone May 26, 2019
@koolzz koolzz deleted the nf_tag_fix branch June 7, 2019 23:17
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