sdnfv / openNetVM

A high performance container-based NFV platform from GW and UCR.
http://sdnfv.github.io/onvm/
Other
263 stars 136 forks source link

onvm_pkt_parse_ip fix #104

Closed dennisafa closed 5 years ago

dennisafa commented 5 years ago

We've been parsing string based IP's in reverse order, this aims to fix that functionality and make modifications to NF's using this function as needed.

Summary:

Previously, we have been converting IP's in string form to IPv4 format in reverse order (IPv4(ip[3], ip[2], ip[1], ip[0]) this PR modifies this to parse in proper format (IPv4(ip[0], ip[1], ip[2], ip[3])

Usage:

This PR includes
Resolves issues
Breaking API changes
Internal API changes X
Usability improvements
Bug fixes X
New functionality
New NF/onvm_mgr args
Changes to starting NFs X
Dependency updates
Web stats updates

Merging notes:

TODO before merging :

Test Plan:

Tested this with Pktgen with nf_router running. Assured that IP address being sent from Pktgen matched the properly parsed IP address in the route.conf file.

Review:

onvm commented 5 years ago

In response to PR creation

CI Message

Your results will arrive shortly

onvm commented 5 years ago

In response to PR creation

CI Message

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

Linter Failed

examples/aes_decrypt/aes.h:176: #endif line should be "#endif // _AESH" [build/header_guard] [5] Total errors found: 1 examples/aes_encrypt/aes.h:185: #endif line should be "#endif // _AESH" [build/header_guard] [5] Total errors found: 1 examples/flow_table/flow_table.h:63: #endif line should be "#endif // _FLOW_TABLEH" [build/header_guard] [5] Total errors found: 1 examples/flow_table/msgbuf.h:71: #endif line should be "#endif // _MSGBUFH" [build/header_guard] [5] Total errors found: 1 examples/flow_table/openflow.h:969: #endif line should be "#endif // _OPENFLOWH" [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 // _SDNH" [build/header_guard] [5] Total errors found: 1 examples/flow_table/sdn_pkt_list.h:122: #endif line should be "#endif // _SDN_PKT_LISTH" [build/header_guard] [5] Total errors found: 1 examples/flow_table/setupconn.h:53: #endif line should be "#endif // _SETUPCONNH" [build/header_guard] [5] Total errors found: 1 examples/nf_router/nf_router.c:173: Lines should be <= 120 characters long [whitespace/line_length] [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:366: #endif line should be "#endif // _ONVM_COMMONH" [build/header_guard] [5] Total errors found: 1 onvm/onvm_nflib/onvm_config_common.h:208: #endif line should be "#endif // _ONVM_CONFIG_COMMONH" [build/header_guard] [5] Total errors found: 1 onvm/onvm_nflib/onvm_msg_common.h:61: #endif line should be "#endif // _ONVM_MSG_COMMONH" [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_COMMONH" [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_MGRH" [build/header_guard] [5] Total errors found: 1

koolzz commented 5 years ago

As briefly discussed don't overwrite the packet data, just use the function when you need to compare.

dennisafa commented 5 years ago

As brifly discussed don't overwrite the packet data, just use the function when you need to compare.

Fixed, looking at other NF's doing the comparison and updating accordingly

twood02 commented 5 years ago

is your ordering now the same as in this code? https://github.com/FOXNEOAdvancedTechnology/MinimalDPDKExamples/blob/master/minimal_tx/minimal_tx.c#L68

dennisafa commented 5 years ago

is your ordering now the same as in this code? https://github.com/FOXNEOAdvancedTechnology/MinimalDPDKExamples/blob/master/minimal_tx/minimal_tx.c#L68

Yes, it is the same. It follows what the IPv4 function is doing here: http://doc.dpdk.org/api/rte__ip_8h.html#a052e2a96de0562a034776fa758255650

dennisafa commented 5 years ago

Should be good to go

koolzz commented 5 years ago

Just in case @onvm

onvm commented 5 years ago

Just in case @onvm

CI Message

Your results will arrive shortly

onvm commented 5 years ago

Just in case @onvm

CI Message

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

Linter Failed

examples/aes_decrypt/aes.h:176: #endif line should be "#endif // _AESH" [build/header_guard] [5] Total errors found: 1 examples/aes_encrypt/aes.h:185: #endif line should be "#endif // _AESH" [build/header_guard] [5] Total errors found: 1 examples/arp_response/arp_response.c:284: Lines should be <= 120 characters long [whitespace/line_length] [5] Total errors found: 1 examples/flow_table/flow_table.h:63: #endif line should be "#endif // _FLOW_TABLEH" [build/header_guard] [5] Total errors found: 1 examples/flow_table/msgbuf.h:71: #endif line should be "#endif // _MSGBUFH" [build/header_guard] [5] Total errors found: 1 examples/flow_table/openflow.h:969: #endif line should be "#endif // _OPENFLOWH" [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 // _SDNH" [build/header_guard] [5] Total errors found: 1 examples/flow_table/sdn_pkt_list.h:122: #endif line should be "#endif // _SDN_PKT_LISTH" [build/header_guard] [5] Total errors found: 1 examples/flow_table/setupconn.h:53: #endif line should be "#endif // _SETUPCONNH" [build/header_guard] [5] Total errors found: 1 examples/nf_router/nf_router.c:173: Lines should be <= 120 characters long [whitespace/line_length] [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:366: #endif line should be "#endif // _ONVM_COMMONH" [build/header_guard] [5] Total errors found: 1 onvm/onvm_nflib/onvm_config_common.h:208: #endif line should be "#endif // _ONVM_CONFIG_COMMONH" [build/header_guard] [5] Total errors found: 1 onvm/onvm_nflib/onvm_msg_common.h:61: #endif line should be "#endif // _ONVM_MSG_COMMONH" [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_COMMONH" [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_MGRH" [build/header_guard] [5] Total errors found: 1