-
Notifications
You must be signed in to change notification settings - Fork 109
[NFV] OVS DPDK SRIOV IPV6 with 1 or 2 nodesets scenarios #660
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
base: main
Are you sure you want to change the base?
Conversation
dcd3c5e to
8cd7e21
Compare
b07ffb5 to
f97984c
Compare
|
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/2f91bf54d0db4b6cbfca37fbbc74c932 ✔️ noop SUCCESS in 0s |
|
recheck |
|
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/160c2efa99e5423597fe38340a7365ae ✔️ noop SUCCESS in 0s |
|
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/db9dc39a0fe24eb1ab2730dda6867641 ✔️ noop SUCCESS in 0s |
|
rdoproject.org/github-check is failing because there is a dependency with this PR in ci-framework. I need to merge in first place ci-framework PR and then this architecture PR |
|
waiting on openstack-k8s-operators/ci-framework#3476 |
|
recheck |
|
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/b0b6ef44390345e0b5beb4a24e7df0a8 ✔️ noop SUCCESS in 0s |
|
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/8a64eca3dd664cab9c02755796875f0f ✔️ noop SUCCESS in 0s |
|
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/a1c7151e396c48489df3840582505931 ✔️ noop SUCCESS in 0s |
|
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/93c9d62fd0ee45639b259b5de374c397 ✔️ noop SUCCESS in 0s |
|
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/18532a3574464381b18ef43e321259bc ✔️ noop SUCCESS in 0s |
|
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/2dc465459d2c4b8488d95434d57f5e1e ✔️ noop SUCCESS in 0s |
|
@mnietoji I see you made some changes. Feel free to remove the "do not merge" label and add the "ready for review" label when you need me to review it again. |
891c045 to
0478dd1
Compare
|
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/fd3f3beea66147ae9c223c06227e8de6 ✔️ noop SUCCESS in 0s |
| ``` | ||
| Change to the nfv/ovs-dpdk-sriov/edpm directory | ||
| ``` | ||
| cd architecture/examples/va/nfv/ovs-dpdk-sriov/edpm |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| cd architecture/examples/va/nfv/ovs-dpdk-sriov/edpm | |
| cd examples/va/nfv/ovs-dpdk-sriov/edpm |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be the v6 path?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am checking documentation in other examples and paths start with the architecture repo that contains the repo, I would not modify it to have same format than the other ones
It is not v6 path because edpm it is being reused
| ``` | ||
| Change to the nfv/ovs-dpdk-sriov directory | ||
| ``` | ||
| cd architecture/examples/va/nfv/ovs-dpdk-sriov |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| cd architecture/examples/va/nfv/ovs-dpdk-sriov | |
| cd examples/va/nfv/ovs-dpdk-sriov |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This example is in examples/va/nfv/ovs-dpdk-sriov-ipv6/
Shouldn't they be doing the following instead?
cd examples/va/nfv/ovs-dpdk-sriov-ipv6
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, updated
fultonj
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a big improvement over the original PR as it has about half as much code (~900 lines, not 1800 lines).
- The steps in the control-plane.md files make sense to me for the v6 scenarios if you update the paths to use the v6 scenarios (I could follow the md file to create the CRs)
- The steps in the data-plane.md files do not make sense to me since the v6 scenarios do not contain an edpm directory. (I could not follow the md file to create the CRs)
I have more detailed questions below.
| ``` | ||
| Change to the nfv/ovs-dpdk-sriov directory | ||
| ``` | ||
| cd architecture/examples/va/nfv/ovs-dpdk-sriov |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This example is in examples/va/nfv/ovs-dpdk-sriov-ipv6/
Shouldn't they be doing the following instead?
cd examples/va/nfv/ovs-dpdk-sriov-ipv6
| ``` | ||
| Change to the nfv/nfv-ovs-dpdk-sriov-2nodesets directory | ||
| ``` | ||
| cd examples/dt/nfv/nfv-ovs-dpdk-sriov-2nodesets |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be the v6 path?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, control-plane routes are wrong, updated
| ``` | ||
| Change to the nfv/nfv-ovs-dpdk-sriov-2nodesets/edpm directory | ||
| ``` | ||
| cd examples/dt/nfv/nfv-ovs-dpdk-sriov-2nodesets/edpm |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be the v6 path?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR adds the nfv-ovs-dpdk-sriov-ipv6-2nodesets directory. The readme files do not say to use this directory though. I originally thought this was a lack of editing after doing a copy/paste, but I also see that the edpm directory does not exist for the added nfv-ovs-dpdk-sriov-ipv6-2nodesets directory.
Are you sure this readme file is correct? In the 2-nodeset v6 sceario do they still use the v4 edpm directory? If so how and when do they use the nfv-ovs-dpdk-sriov-ipv6-2nodesets directory?
This should be clear from reading the .md files but it's not clear to me right now since I have the above questions.
[fultonj@stybba nfv{nfv_ipv6}]$ ll
total 4.0K
drwxr-xr-x. 4 fultonj fultonj 162 Jan 6 10:27 nfv-ovs-dpdk-sriov-networker
drwxr-xr-x. 5 fultonj fultonj 4.0K Jan 6 10:27 nfv-ovs-dpdk-sriov-hci
drwxr-xr-x. 4 fultonj fultonj 162 Jan 6 10:27 nfv-ovs-dpdk-sriov-2nodesets
drwxr-xr-x. 3 fultonj fultonj 150 Jan 12 15:18 nfv-ovs-dpdk-sriov-ipv6-2nodesets
[fultonj@stybba nfv{nfv_ipv6}]$ find . -name edpm
./nfv-ovs-dpdk-sriov-2nodesets/edpm
./nfv-ovs-dpdk-sriov-networker/edpm
[fultonj@stybba nfv{nfv_ipv6}]$
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated readme
| ``` | ||
| Change to the nfv/ovs-dpdk-sriov/edpm directory | ||
| ``` | ||
| cd architecture/examples/va/nfv/ovs-dpdk-sriov/edpm |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be the v6 path?
Created 2 new scenarios adding ipv6 to 2 already existing scenarios: * VA OVS DPDK SRIOV * DT OVS DPDK SRIOV with 2 nodesets Signed-off-by: Miguel Angel Nieto Jimenez <[email protected]>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: mnietoji The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
There were some issues in the documentation, updated:
|
fultonj
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
[NFV] OVS DPDK SRIOV IPV6 with 1 or 2 nodesets scenarios
Created 2 new scenarios adding ipv6 to 2 already existing scenarios: