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

Initial integration of Cloud Native Data Plane with BESS OMEC-UPF #656

Merged
merged 4 commits into from Jun 12, 2023

Conversation

manojgop
Copy link
Contributor

@manojgop manojgop commented Dec 8, 2022

  • Add support for Cloud Native Data Plane (CNDP) with BESS UPF pipeline.
  • Modify Dockerfile and scripts to add support of CNDP.
  • Example CNDP JSONC configuration files for single and multiple worker threads.
  • README file with instructions for setting up BESS UPF with CNDP.

Signed-off-by: Manoj Gopalakrishnan manoj.gopalakrishnan@intel.com

@omecproject
Copy link

Can one of the admins verify this patch?

@manojgop
Copy link
Contributor Author

manojgop commented Dec 8, 2022

This PR should be merged after BESS PR omec-project/bess#2 is merged

@manojgop manojgop force-pushed the feat_cndp_upf_initial branch 2 times, most recently from 04716e2 to 2854309 Compare December 8, 2022 06:22
@manojgop
Copy link
Contributor Author

CI pipeline mentions there is no copyright and licensing information for image file in this PR. Do we need to add these details for image files. If so, how can we add that ?

The following files have no copyright and licensing information:
* docs/images/cndp-omec-upf-test-setup.jpg

@gab-arrobo
Copy link
Collaborator

CI pipeline mentions there is no copyright and licensing information for image file in this PR. Do we need to add these details for image files. If so, how can we add that ?

The following files have no copyright and licensing information:
* docs/images/cndp-omec-upf-test-setup.jpg

@amolonf @thakurajayL please your comments/feedback about this. Thanks!

@gab-arrobo
Copy link
Collaborator

CI pipeline mentions there is no copyright and licensing information for image file in this PR. Do we need to add these details for image files. If so, how can we add that ?

The following files have no copyright and licensing information:
* docs/images/cndp-omec-upf-test-setup.jpg

@amolonf @thakurajayL please your comments/feedback about this. Thanks!

Hi @amolonf @thakurajayL , any comments about this. Thanks!

@amolonf
Copy link
Contributor

amolonf commented Jan 14, 2023

retest this please

@gab-arrobo
Copy link
Collaborator

@amolonf, the issue is that one of the checks related to the copyrights fails because an image (JPG file) does not have a copyright. Please see it here:

The following files have no copyright and licensing information:

  • docs/images/cndp-omec-upf-test-setup.jpg

@amolonf
Copy link
Contributor

amolonf commented Jan 14, 2023

@amolonf, the issue is that one of the checks related to the copyrights fails because an image (JPG file) does not have a copyright. Please see it here:

The following files have no copyright and licensing information:

  • docs/images/cndp-omec-upf-test-setup.jpg

I did not see this kind of error. Can you please try with other format like svg?

@gab-arrobo
Copy link
Collaborator

@amolonf, the issue is that one of the checks related to the copyrights fails because an image (JPG file) does not have a copyright. Please see it here:

The following files have no copyright and licensing information:

  • docs/images/cndp-omec-upf-test-setup.jpg

I did not see this kind of error. Can you please try with other format like svg?

@manojgop, can you please try Amol's suggestion?

@gab-arrobo
Copy link
Collaborator

CI pipeline mentions there is no copyright and licensing information for image file in this PR. Do we need to add these details for image files. If so, how can we add that ?

The following files have no copyright and licensing information:
* docs/images/cndp-omec-upf-test-setup.jpg

@manojgop you can use reuse to do that. Here are the instructions: https://reuse.software/tutorial/#step-2

Copy link
Collaborator

@gab-arrobo gab-arrobo left a comment

Choose a reason for hiding this comment

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

Need to address issue with copyright in one file

@manojgop
Copy link
Contributor Author

Need to address issue with copyright in one file

Reuse tool created "cndp-omec-upf-test-setup.jpg.license" file with below content. It didn't modify the original jpg file. So should I include the file "cndp-omec-upf-test-setup.jpg.license" in PR

Copyright 2019 Intel Corporation

SPDX-License-Identifier: Apache-2.0

@gab-arrobo
Copy link
Collaborator

gab-arrobo commented Jan 21, 2023 via email

pfcpiface/config.go Outdated Show resolved Hide resolved
scripts/docker_setup.sh Outdated Show resolved Hide resolved
scripts/docker_setup.sh Outdated Show resolved Hide resolved
scripts/docker_setup.sh Outdated Show resolved Hide resolved
scripts/docker_setup.sh Outdated Show resolved Hide resolved
conf/pktgen_cndp.bess Show resolved Hide resolved
conf/upf.json Outdated Show resolved Hide resolved
conf/upf.json Outdated Show resolved Hide resolved
@manojgop
Copy link
Contributor Author

@gab-arrobo Have updated PR incorporating review comments. Please review

@gab-arrobo
Copy link
Collaborator

@gab-arrobo Have updated PR incorporating review comments. Please review

Hi @manojgop, I will try to give it another review this week.

@manojgop
Copy link
Contributor Author

@gab-arrobo Have updated PR incorporating review comments. Please review

Hi @manojgop, I will try to give it another review this week.

@gab-arrobo Did you get a chance to review

@gab-arrobo
Copy link
Collaborator

@gab-arrobo Have updated PR incorporating review comments. Please review

Hi @manojgop, I will try to give it another review this week.

@gab-arrobo Did you get a chance to review

@manojgop. I had not had time to review it again... I am going to try to give it another review this or next week. Sorry for the delay!

docs/CNDP_README.md Outdated Show resolved Hide resolved
conf/cndp_upf_4worker.jsonc Show resolved Hide resolved
conf/cndp_upf_1worker.jsonc Outdated Show resolved Hide resolved
conf/cndp_upf_1worker.jsonc Outdated Show resolved Hide resolved
conf/cndp_upf_4worker.jsonc Outdated Show resolved Hide resolved
conf/upf.json Outdated Show resolved Hide resolved
scripts/docker_setup.sh Outdated Show resolved Hide resolved
scripts/docker_setup.sh Outdated Show resolved Hide resolved
scripts/docker_setup.sh Outdated Show resolved Hide resolved
scripts/docker_setup.sh Outdated Show resolved Hide resolved
@manojgop manojgop force-pushed the feat_cndp_upf_initial branch 2 times, most recently from 4b02396 to bd725d2 Compare March 14, 2023 10:41
@manojgop
Copy link
Contributor Author

manojgop commented Apr 18, 2023

Can anyone explain why there is a PMD option for xskdev API? image_2023-04-14_05-39-14

reference: image

isn't this option only apply to pktdev API? image

CNDP supports different poll mode drivers (PMD). For BESS UPF, CNDP uses AF-XDP PMD. More info in https://github.com/CloudNativeDataPlane/cndp/blob/main/doc/guides/pmds/overview.rst. And CNDP - BESS uses xskdev to use custom mempool management for AF-XDP UMEM

@manojgop manojgop force-pushed the feat_cndp_upf_initial branch 2 times, most recently from 4a178f8 to 05492e2 Compare April 18, 2023 16:10
@gab-arrobo
Copy link
Collaborator

@badhrinathpa @thakurajayL @amarsri28 @sureshmarikkannu, I would appreciate your input about this PR

docs/CNDP_README.md Outdated Show resolved Hide resolved
Comment on lines 14 to 29
### Step 1: Build the OMEC UPF docker container

> Note: If you are behind a proxy make sure to export/setenv http_proxy and https_proxy

From the top level directory call:

```
$ make docker-build
```
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this step really needed? make docker-build is already being called from docker_setup.sh

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Building the image upfront will make script execution faster. I could remove it if you feel this additional step is not user friendly.

docs/CNDP_README.md Outdated Show resolved Hide resolved
docs/CNDP_README.md Outdated Show resolved Hide resolved
docs/CNDP_README.md Outdated Show resolved Hide resolved
docs/CNDP_README.md Outdated Show resolved Hide resolved
docs/CNDP_README.md Outdated Show resolved Hide resolved
docs/CNDP_README.md Outdated Show resolved Hide resolved
docs/CNDP_README.md Outdated Show resolved Hide resolved
docs/CNDP_README.md Outdated Show resolved Hide resolved
docs/CNDP_README.md Outdated Show resolved Hide resolved
docs/CNDP_README.md Outdated Show resolved Hide resolved
docs/CNDP_README.md Outdated Show resolved Hide resolved
docs/CNDP_README.md Outdated Show resolved Hide resolved
docs/CNDP_README.md Outdated Show resolved Hide resolved
docs/CNDP_README.md Outdated Show resolved Hide resolved
docs/CNDP_README.md Outdated Show resolved Hide resolved
docs/CNDP_README.md Outdated Show resolved Hide resolved
docs/CNDP_README.md Outdated Show resolved Hide resolved
docs/CNDP_README.md Outdated Show resolved Hide resolved
docs/CNDP_README.md Outdated Show resolved Hide resolved
scripts/cndp_reset_upf.sh Outdated Show resolved Hide resolved
scripts/cndp_reset_upf.sh Outdated Show resolved Hide resolved
docs/CNDP_README.md Outdated Show resolved Hide resolved
docs/CNDP_README.md Outdated Show resolved Hide resolved
@gab-arrobo
Copy link
Collaborator

@manojgop, BTW, is not there need to enable DDP?

docs/CNDP_README.md Outdated Show resolved Hide resolved
docs/CNDP_README.md Outdated Show resolved Hide resolved
docs/CNDP_README.md Outdated Show resolved Hide resolved
docs/CNDP_README.md Outdated Show resolved Hide resolved
docs/CNDP_README.md Outdated Show resolved Hide resolved
docs/CNDP_README.md Outdated Show resolved Hide resolved
docs/CNDP_README.md Outdated Show resolved Hide resolved
@manojgop manojgop force-pushed the feat_cndp_upf_initial branch 3 times, most recently from e889d22 to 863f3c5 Compare May 30, 2023 12:29
- Add support for Cloud Native Data Plane (CNDP) with BESS UPF pipeline.
- Modify Dockerfile and scripts to add support of CNDP.
- Example CNDP JSONC configuration files for single and multiple worker threads.
- README file with instructions for setting up BESS UPF with CNDP.

Signed-off-by: Manoj Gopalakrishnan <manoj.gopalakrishnan@intel.com>
Signed-off-by: Manoj Gopalakrishnan <manoj.gopalakrishnan@intel.com>
scripts/docker_setup.sh Outdated Show resolved Hide resolved
docs/CNDP_README.md Outdated Show resolved Hide resolved
docs/CNDP_README.md Outdated Show resolved Hide resolved
Signed-off-by: Manoj Gopalakrishnan <manoj.gopalakrishnan@intel.com>
docs/CNDP_README.md Outdated Show resolved Hide resolved
docs/CNDP_README.md Outdated Show resolved Hide resolved
@gab-arrobo
Copy link
Collaborator

@manojgop, everything seems to be working as expected. Please added the issue with the Speellcheck GHA and we will be done with this review. Thanks!

@manojgop
Copy link
Contributor Author

manojgop commented Jun 8, 2023

@manojgop, everything seems to be working as expected. Please added the issue with the Speellcheck GHA and we will be done with this review. Thanks!

Updated PR

Signed-off-by: Manoj Gopalakrishnan <manoj.gopalakrishnan@intel.com>
Copy link
Collaborator

@gab-arrobo gab-arrobo left a comment

Choose a reason for hiding this comment

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

+1

@gab-arrobo
Copy link
Collaborator

@thakurajayL, @badhrinathpa, @amarsri28, @sureshmarikkannu, I completed the review (including testing it with 1, 4 and 8 workers) and everything in PR looks good. So, I approved it. I am planning to merge it in a couple days. Please feel free to provide your input if possible. Thanks!

@gab-arrobo gab-arrobo merged commit 4181480 into omec-project:master Jun 12, 2023
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants