Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Set virt addr #197

Merged
merged 15 commits into from
Mar 23, 2020
Merged

Set virt addr #197

merged 15 commits into from
Mar 23, 2020

Conversation

bdevierno1
Copy link
Contributor

Default base virtual address set. Readme Updated

Resolves issue 186 and 100. Added documentation to reflect default address set.

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

TODO before merging :

  • [ 👍 ] PR is ready for review

Test Plan:

Address can be updated by user. Tested with different size huge pages.

Review:

Readme Documentation
@dennisafa

@onvm
Copy link

onvm commented Mar 19, 2020

In response to PR creation

CI Message

Your results will arrive shortly

onvm
onvm previously approved these changes Mar 19, 2020
Copy link

@onvm onvm left a comment

Choose a reason for hiding this comment

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

In response to PR creation

CI Message

Run successful see results:
✔️ PR submitted to develop branch
✔️ Linter passed

@kevindweb
Copy link
Contributor

kevindweb commented Mar 19, 2020

I was wondering where this went lol, good work 👍

koolzz
koolzz previously approved these changes Mar 19, 2020
onvm/go.sh Outdated
# If base virtual address has not been set by the user, set to default.
if [[ -z $virt_addr ]]
then
echo "Base virtual address set to default"
Copy link
Member

Choose a reason for hiding this comment

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

[nit] might as well print what the default virt addr is, nobody really reads the usage. Looks good tho

dennisafa
dennisafa previously approved these changes Mar 19, 2020
Copy link
Member

@dennisafa dennisafa left a comment

Choose a reason for hiding this comment

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

good work! resolve nicks comment and should be fine to merge

Print default address that has been set to user.
Copy link
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.

Code looks good, let's run with @onvm

@kevindweb
Copy link
Contributor

Well I guess @onvm doesn't run on reviews

@onvm
Copy link

onvm commented Mar 22, 2020

Well I guess @onvm doesn't run on reviews

CI Message

Your results will arrive shortly

Copy link

@onvm onvm left a comment

Choose a reason for hiding this comment

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

Well I guess @onvm doesn't run on reviews

CI Message

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

[Results from nimbnode23]

  • Median TX pps for Pktgen: 7710544
    Performance rating - 100.14% (compared to 7700000 average)

  • Median TX pps for Speed Tester: 41769838
    Performance rating - 99.45% (compared to 42000000 average)

Copy link
Member

@dennisafa dennisafa left a comment

Choose a reason for hiding this comment

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

looking good to me. good work!

Copy link
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.

Lgtm, take it away @dennisafa

@dennisafa dennisafa merged commit 584b980 into sdnfv:develop Mar 23, 2020
@bdevierno1 bdevierno1 deleted the set-virt-addr branch April 7, 2020 01:31
@twood02 twood02 added this to the ONVM 20.05 milestone Apr 9, 2020
@dennisafa dennisafa mentioned this pull request May 3, 2020
3 tasks
@twood02 twood02 added NeedsReleaseNote Needs updated release note info and removed NeedsReleaseNote Needs updated release note info labels May 22, 2020
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.

Error while setting up envt. ./onvm/go.sh 1,2,3 1 -s stdout -a 0x7f000000000 stop running
6 participants