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

Go binding cleanup and example app #154

Merged
merged 4 commits into from
Aug 23, 2022

Conversation

KeithWiles
Copy link
Contributor

@KeithWiles KeithWiles commented Aug 15, 2022

Here is a list on what I cleaned up:

  • Simplify the usability and utilize the full JSON-c configuration.
  • Remove some files that are combined into others and rework a few areas to make it easier to understand.
  • Rework RxBurst/TxBurst/… to take a lportID instead of passing a method pointer.
  • Cleaned up a number of allocations around LPort as this data is static after startup.
  • Remove all of the underscore variables and convert them to Go variable names.
  • Add the start of documentation to all of the files. Needs more work.
  • Added a few more helper routines to parse a slice to packets or objects.
  • Update the example/fwd application to utilize the JSON-c instead of JSON
  • Remove the -port option and use the JSON-c thread section to identify the lports attached to a given thread
  • Add support for using the JSON Threads section for starting threads an assigning lports to those threads.
  • Use the tview Go library to display the stats in a easier to read format.
  • Update the test routines to poll more than just one lport.
  • Updated the cne/run_test code and script to test more functions.
  • Updated the example/run_fwd script remove the -port option with other cleanups.
  • More minor changes for refactoring and cleanup.

Signed-off-by: Keith Wiles keith.wiles@intel.com

@KeithWiles
Copy link
Contributor Author

The super linter is executing on the XXX_test.go files and it should not unless we can figure out how to make it work. I do not think is a big problem, so I would like to figure out how to exclude *_test.go files.

The next problem is the Super-Linter does not appear to understand import "C" statements and that causes a number of failures.

@KeithWiles
Copy link
Contributor Author

This PR needs #153 to be complete, but I could not find how to make this PR dependent on PR 153.

@elzamath
Copy link
Contributor

elzamath commented Aug 15, 2022

The super linter is executing on the XXX_test.go files and it should not unless we can figure out how to make it work. I do not think is a big problem, so I would like to figure out how to exclude *_test.go files.

The next problem is the Super-Linter does not appear to understand import "C" statements and that causes a number of failures.

setting VALIDATE_GO: false https://github.com/CloudNativeDataPlane/cndp/blob/main/.github/workflows/super-linter.yml#L33 would prevent Go files from being linted

These issues seem to be related to the failures shown by the Go linter
golangci/golangci-lint#1176 related to the

could not import C (cgo preprocessing failed) (typecheck)
import "C"

https://github.com/github/super-linter/issues/1599 - related to the the undeclared name failure

Copy link
Contributor

@maryamtahhan maryamtahhan left a comment

Choose a reason for hiding this comment

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

Just a start....
got as far as lport_test.go (will keep tipping away at it)

lang/go/bindings/cne/jcfg.go Outdated Show resolved Hide resolved
lang/go/bindings/cne/jcfg.go Outdated Show resolved Hide resolved
lang/go/bindings/cne/jcfg.go Outdated Show resolved Hide resolved
lang/go/bindings/cne/lport.go Outdated Show resolved Hide resolved
lang/go/bindings/cne/lport.go Outdated Show resolved Hide resolved
lang/go/bindings/cne/lport.go Outdated Show resolved Hide resolved
lang/go/bindings/cne/lport.go Outdated Show resolved Hide resolved
lang/go/bindings/cne/lport.go Show resolved Hide resolved
lang/go/bindings/cne/lport.go Show resolved Hide resolved
Copy link
Contributor

@maryamtahhan maryamtahhan left a comment

Choose a reason for hiding this comment

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

got as far as the examples dir - will try to close off the rest tomorrow

lang/go/bindings/cne/lport_test.go Show resolved Hide resolved
lang/go/bindings/cne/lport_test.go Outdated Show resolved Hide resolved
lang/go/bindings/cne/lport_test.go Outdated Show resolved Hide resolved
lang/go/bindings/cne/lring.go Show resolved Hide resolved
lang/go/bindings/cne/msgchan.go Outdated Show resolved Hide resolved
lang/go/bindings/cne/jcfg.go Outdated Show resolved Hide resolved
lang/go/bindings/cne/system.go Show resolved Hide resolved
lang/go/bindings/cne/system_test.go Show resolved Hide resolved
lang/go/bindings/cne/system_test.go Show resolved Hide resolved
lang/go/bindings/cne/umem.go Show resolved Hide resolved
Here is a list on what I cleaned up (not a complete list):
  - Simplify the usability and utilize the full JSON-c configuration.
  - Remove some files that are combined into others and rework a few areas to
    make it easier to understand.
  - Rework RxBurst/TxBurst/… to take a lportID instead of passing a method pointer.
  - Cleaned up a number of allocations around LPort as this data is static after startup.
  - Remove all of the underscore variables and convert them to Go variable names.
  - Add the start of documentation to all of the files. Needs more work.
  - Added a few more helper routines to parse a slice to packets or objects.
  - Update the example/fwd application to utilize the JSON-c instead of JSON
  - Remove the -port option and use the JSON-c thread section to identify the lports
    attached to a given thread
  - Add support for using the JSON Threads section for starting threads an assigning
    lports to those threads.
  - Use the tview Go library to display the stats in a easier to read format.
  - Update the test routines to poll more than just one lport.
  - Updated the cne/run_test code and script to test more functions.
  - Updated the example/run_fwd script remove the -port option with other cleanups.
  - Fix the tx only packet format as it was missing part of the packet data
  - More minor changes for refactoring and cleanup.

Signed-off-by: Keith Wiles <keith.wiles@intel.com>
@KeithWiles
Copy link
Contributor Author

KeithWiles commented Aug 21, 2022

Are we all done reviewing this one?
@maryamtahhan @jeffreybshaw @sushmasi

Thank you for the reviews.

@jeffreybshaw
Copy link
Contributor

If @maryamtahhan approves, let's merge this.

Copy link
Contributor

@maryamtahhan maryamtahhan left a comment

Choose a reason for hiding this comment

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

sorry for the delay. LGTM

@maryamtahhan maryamtahhan merged commit 1ee0cb5 into CloudNativeDataPlane:main Aug 23, 2022
@KeithWiles KeithWiles deleted the go_binding_update branch August 23, 2022 01:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants