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

[Edgecore][as9516-32d] Access BMC by using libcurl #941

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

WillyLiu-EC
Copy link
Contributor

@WillyLiu-EC WillyLiu-EC commented May 31, 2023

[Edgecore][as9516-32d] Access BMC by using libcurl

  1. Modify kernel config
    This modification is for creating usb0 interface.
    And then, it can access BMC through usb0
  2. Implement the BMC curl accessing for ONL
    To make sure BMC response, it uses libcurl to access BMC.
  3. Fix the issus that failed with build ONL
    It can not build ONL for master. Need to modify the debian path for stretch.

Copy link

@paulmenzel paulmenzel left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution.

Please fix the typos and formatting of the merge/pull request description. Please also describe the goal. Where is OpenBMCv1 installed on?

@@ -29,14 +29,14 @@ Multistrap:

Debian:
packages: *Packages
source: http://${DEBIAN_MIRROR}
source: http://archive.debian.org/debian/

Choose a reason for hiding this comment

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

This looks unrelated.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We git clone the codebase from community and build it. The building result is failed.
We reference the this modification to make sure the success building.
https://github.com/opencomputeproject/OpenNetworkLinux/pull/933/files

Although this modification actually is unrelated for as9516-32d platform, the ONL installer need to build successfully.

We can close this PR and send the two PR(one for building, one for as9516-32d) if need be.

Choose a reason for hiding this comment

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

We git clone the codebase from community and build it. The building result is failed.
We reference the this modification to make sure the success building. https://github.com/opencomputeproject/OpenNetworkLinux/pull/933/files

You should then merge that branch, so the relationship is clear.

We can close this PR and send the two PR(one for building, one for as9516-32d) if need be.

No need to close this one. Just “force-push” your amended commits.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We git clone the codebase from community and build it. The building result is failed.
We reference the this modification to make sure the success building. https://github.com/opencomputeproject/OpenNetworkLinux/pull/933/files

You should then merge that branch, so the relationship is clear.

We can close this PR and send the two PR(one for building, one for as9516-32d) if need be.

No need to close this one. Just “force-push” your amended commits.

This PR is related jessie and wheezy, not include stretch. And this PR provider is not us.
https://github.com/opencomputeproject/OpenNetworkLinux/pull/933/files

For the clear relationship, should we send a new PR for stretch ?
And then remove this stretch modification from this PR ?

@WillyLiu-EC
Copy link
Contributor Author

Thank you for your contribution.

Please fix the typos and formatting of the merge/pull request description. Please also describe the goal. Where is OpenBMCv1 installed on?

Thank you for your suggestion.
About this question, maybe we modify the branch name as9516-32d_libcrul_with_openbmc_v1 to as9516-32d_libcrul_with_bmc.
The main modifications is using libcurl to access bmc.

@paulmenzel
Copy link

In my opinion the branch name is less important than the lost discussion/comments of the merge/pull request.

Should you change it, please spell curl correctly. ;-)

@WillyLiu-EC
Copy link
Contributor Author

In my opinion the branch name is less important than the lost discussion/comments of the merge/pull request.

Should you change it, please spell curl correctly. ;-)

Sorry about wrong branch name.
The correct branch name: as9516-32d_libcurl_with_bmc

1. Modify kernel config for using usb0 interface
2. Implement the BMC curl accessing for ONL
3. Fix the issus that failed building with stretch

Signed-off-by: Willy Liu <[email protected]>
@WillyLiu-EC WillyLiu-EC force-pushed the as9516-32d_libcrul_with_openbmc_v1 branch from 95be6f8 to 5993f24 Compare June 1, 2023 07:30
@WillyLiu-EC WillyLiu-EC changed the title As9516 32d libcrul with openbmc v1 [AS9516-32D] Access bmc by using libcurl Jun 1, 2023
@WillyLiu-EC
Copy link
Contributor Author

In my opinion the branch name is less important than the lost discussion/comments of the merge/pull request.

Should you change it, please spell curl correctly. ;-)

OK, we already modify it.
please check it.

Best regards

@WillyLiu-EC WillyLiu-EC changed the title [AS9516-32D] Access bmc by using libcurl [AS9516-32D] Access BMC by using libcurl Jun 1, 2023
@paulmenzel
Copy link

Thank you for the improvements. Still some requests:

  1. Please split the commit into one commit per item you mentioned with a commit message. (Answering for example the question: „Why is the usb0 interface needed for BMC?“.)
  2. Please do not indent the merge/pull request description with four spaces. Currently GitHub marks it up as a code block (monospace).

@WillyLiu-EC WillyLiu-EC changed the title [AS9516-32D] Access BMC by using libcurl [Edgecore][as9516-32d] Access BMC by using libcurl Jun 1, 2023
@WillyLiu-EC
Copy link
Contributor Author

Thank you for the improvements. Still some requests:

  1. Please split the commit into one commit per item you mentioned with a commit message. (Answering for example the question: „Why is the usb0 interface needed for BMC?“.)
  2. Please do not indent the merge/pull request description with four spaces. Currently GitHub marks it up as a code block (monospace).

Thanks for your suggestion.
Please check it.

@sonoble
Copy link
Contributor

sonoble commented Sep 1, 2023

Hi,
I am stepping through the outstanding PRs, this one does not compile when you run "make docker" due to an issue with curl.

make[4]: warning: -jN forced in submake: disabling jobserver mode. LinkingShared[release]: libonlp-platform-defaults-module::libonlp-platform-defaults.so /usr/lib/gcc-cross/aarch64-linux-gnu/6/../../../../aarch64-linux-gnu/bin/ld: cannot find -lcurl collect2: error: ld returned 1 exit status

@WillyLiu-EC
Copy link
Contributor Author

Hi, I am stepping through the outstanding PRs, this one does not compile when you run "make docker" due to an issue with curl.

make[4]: warning: -jN forced in submake: disabling jobserver mode. LinkingShared[release]: libonlp-platform-defaults-module::libonlp-platform-defaults.so /usr/lib/gcc-cross/aarch64-linux-gnu/6/../../../../aarch64-linux-gnu/bin/ld: cannot find -lcurl collect2: error: ld returned 1 exit status

Hi
I download(git clone)a new codebase from this branch. And the the building result is successful.
Could you download(git clone) a new codebase to build again ?

Best regards,
WillyLiu

@sonoble
Copy link
Contributor

sonoble commented Sep 11, 2023

Hi WillyLiu,

I downloaded the repo and patched but I get the same failure.. are you running "make docker" after you clone and patch?

@WillyLiu-EC
Copy link
Contributor Author

Hi WillyLiu,

I downloaded the repo and patched but I get the same failure.. are you running "make docker" after you clone and patch?

Our building process

  1. docker/tools/onlbuilder -9
  2. source setup.env
  3. make amd64

@sonoble
Copy link
Contributor

sonoble commented Sep 12, 2023 via email

@WillyLiu-EC
Copy link
Contributor Author

New mail: [email protected]

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.

3 participants