onvm
commented
3 years ago In response to PR creation
CI Message
Aborting, need an authorized user to run CI
Closed NoahChinitz closed 3 years ago
onvm
commented
3 years ago In response to PR creation
Aborting, need an authorized user to run CI
twood02
commented
3 years ago @NoahChinitzGWU - to keep our PRs small and targeted (and to force you to practice your git), can you separate out your documentation edits since they aren't directly tied to the rest of this PR?
You can make a separate PR for the doc updates. Then we will decide whether to integrate them here or we may switch over to keeping all docs on the readthedocs site, with a simplified install guide here.
bdevierno1
commented
3 years ago Is the bridge changes tied to the new NF as well? I would separate that out as well. Also printing packet metadata in the handler per packet can have an impact on performance due to print overhead. Move this to the do stats display https://github.com/sdnfv/openNetVM/blob/e65f15810f77c6a8d18c99c028515abb08fb6412/examples/bridge/bridge.c#L112 Note printing is done here as well. https://github.com/sdnfv/openNetVM/blob/e65f15810f77c6a8d18c99c028515abb08fb6412/examples/bridge/bridge.c#L133 and that function already checks for the pkt protocol. https://github.com/sdnfv/openNetVM/blob/61ce33a4ca6d3d7662bb091bdd8bcfc9a61c792d/onvm/onvm_nflib/onvm_pkt_helper.c#L214
Small changes were made to Install.md (changed Step 3.5 NIC PCI as well as added another step under "Configure and compile DPDK").
This PR also includes a new NF called Basic Message Test that tests the functionality of an NF sending messages to itself. This NF was created in response to Issue #293 where a memory leak was fixed.
Summary:
Usage: Usage is described in README.md
Merging notes:
TODO before merging :
Test Plan:
The NF was run extensively to ensure no Segmentation Faults occurred, as well as proper allocation (more specifically deallocation) of memory was maintained.
Review: