#
83328316 |
| 09-May-2024 |
Ed Tanous <ed@tanous.net> |
Fix lesser used options
25b54dba775b31021a3a4677eb79e9771bcb97f7 missed several cases where we had ifndef instead of ifdef. because these weren't the defaults, these don't show up as failures when
Fix lesser used options
25b54dba775b31021a3a4677eb79e9771bcb97f7 missed several cases where we had ifndef instead of ifdef. because these weren't the defaults, these don't show up as failures when testing.
Tested: Redfish service validator passes. Inspection primarily. Mechanical change.
Change-Id: I3f6915a97eb44d071795aed76476c6bee7e8ed27 Signed-off-by: Ed Tanous <ed@tanous.net>
show more ...
|
#
102a4cda |
| 15-Apr-2024 |
Jonathan Doman <jonathan.doman@intel.com> |
Manage Request with shared_ptr
This is an attempt to solve a class of use-after-move bugs on the Request objects which have popped up several times. This more clearly identifies code which owns the
Manage Request with shared_ptr
This is an attempt to solve a class of use-after-move bugs on the Request objects which have popped up several times. This more clearly identifies code which owns the Request objects and has a need to keep it alive. Currently it's just the `Connection` (or `HTTP2Connection`) (which needs to access Request headers while sending the response), and the `validatePrivilege()` function (which needs to temporarily own the Request while doing an asynchronous D-Bus call). Route handlers are provided a non-owning `Request&` for immediate use and required to not hold the `Request&` for future use.
Tested: Redfish validator passes (with a few unrelated fails). Redfish URLs are sent to a browser as HTML instead of raw JSON.
Change-Id: Id581fda90b6bceddd08a5dc7ff0a04b91e7394bf Signed-off-by: Jonathan Doman <jonathan.doman@intel.com> Signed-off-by: Ed Tanous <ed@tanous.net>
show more ...
|
#
499b5b4d |
| 06-Apr-2024 |
Ed Tanous <ed@tanous.net> |
Add static webpack etag support
Webpack (which is what vue uses to compress its HTML) is capable of generating hashes of files when it produces the dist files[1].
This gets generated in the form of
Add static webpack etag support
Webpack (which is what vue uses to compress its HTML) is capable of generating hashes of files when it produces the dist files[1].
This gets generated in the form of <filename>.<hash>.<extension>
This commit attempts to detect these patterns, and enable etag caching to speed up webui load times. It detects these patterns, grabs the hash for the file, and returns it in the Etag header[2].
The behavior is implemented such that: If the file has an etag, the etag header is returned. If the request has an If-None-Match header, and that header matches, only 304 is returned.
Tested: Tests were run on qemu S7106 bmcweb with default error logging level, and HTTP/2 enabled, along with svg optimization patches.
Run scripts/generate_auth_certificate.py to set up TLS certificates. (valid TLS certs are required for HTTP caching to work properly in some browsers). Load the webui. Note that DOM load takes 1.10 seconds, Load takes 1.10 seconds, and all requests return 200 OK. Refresh the GUI. Note that most resources now return 304, and DOM time is reduced to 279 milliseconds and load is reduced to 280 milliseconds. DOM load (which is what the BMC has control over) is decreased by a factor of 3-4X. Setting chrome to "Fast 5g" throttling in the network tab shows a more pronounced difference, 1.28S load time vs 3.96S.
BMC also shows 477KB transferred on the wire, versus 2.3KB transferred on the wire. This has the potential to significantly reduce the load on the BMC when the webui refreshes.
[1] https://webpack.js.org/guides/caching/ [2] https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/ETag
Change-Id: I68aa7ef75533506d98e8fce10bb04a494dc49669 Signed-off-by: Ed Tanous <ed@tanous.net>
show more ...
|
#
52c15028 |
| 19-Apr-2024 |
Ed Tanous <ed@tanous.net> |
Fix http2 use after free bug
In the below code, we move out of Response, then use it to set unauthorized, which never gets returned to the user. This results in the browser showing an empty 200 ok
Fix http2 use after free bug
In the below code, we move out of Response, then use it to set unauthorized, which never gets returned to the user. This results in the browser showing an empty 200 ok request, because while the request was propagated rejected, the 401 error code didn't get propagated to the user.
Tested: If not logged in on a chrome browser: /redfish/v1 -> Returns the UI /refish/v1/AccountService -> returns a forward to the webui login page.
If logged into the webui. /redfish/v1/AccountService now returns the expected HTML redfish representation of the json response.
Change-Id: I2c906f818367ebb253b3e6097e6787ba4c215e0a Signed-off-by: Ed Tanous <ed@tanous.net>
show more ...
|
#
003301a2 |
| 16-Apr-2024 |
Ed Tanous <ed@tanous.net> |
Change ssl stream implementations
Boost beast ssl_stream is just a wrapper around asio ssl_stream, and aims to optimize the case where we're writing small payloads (one or two bytes.) which needs to
Change ssl stream implementations
Boost beast ssl_stream is just a wrapper around asio ssl_stream, and aims to optimize the case where we're writing small payloads (one or two bytes.) which needs to be optimized in SSL.
bmcweb never writes one or two bytes, we almost always write the full payload of what we received, so there's no reason to take the binary size overhead, and additional boost headers that this implementation requires.
Tested: This drops the on-target binary size by 2.6%
Redfish service validator passes.
Change-Id: Ie1ae6f197f8e5ed70cf4abc6be9b1b382c42d64d Signed-off-by: Ed Tanous <ed@tanous.net>
show more ...
|
#
8e8245db |
| 12-Apr-2024 |
Ed Tanous <ed@tanous.net> |
Fix nullptr failures for image upload
Several places that call *req.ioService were missing nullptr checks. Add them, and fix the one case where it might not be filled in.
Tested: With HTTP2 enable
Fix nullptr failures for image upload
Several places that call *req.ioService were missing nullptr checks. Add them, and fix the one case where it might not be filled in.
Tested: With HTTP2 enabled, the following command succeeds. ``` curl -k https://192.168.7.2/redfish/v1/UpdateService/update -F 'UpdateParameters={"Targets":["/redfish/v1/Managers/bmc"]} ;type=application/json' --user "root:0penBmc" -F UpdateFile=@/home/ed/bmcweb/16mb.txt -v -H "Expect:" ```
Change-Id: I81e7944c22f5922d461bf5d231086c7468a16e62 Signed-off-by: Ed Tanous <ed@tanous.net>
show more ...
|
#
8e5cc7bd |
| 02-Apr-2024 |
Ed Tanous <ed@tanous.net> |
Fix http2 payloads
d547d8d2c30a7d00852855da8ecc15c0cc424b0e caused a minor regression in a poorly executed merge conflict. The reqReader instance never gets initialized, so all payloads now fail.
Fix http2 payloads
d547d8d2c30a7d00852855da8ecc15c0cc424b0e caused a minor regression in a poorly executed merge conflict. The reqReader instance never gets initialized, so all payloads now fail.
Tested: Redfish service validator modified to use http2 now succeeds when using http/2.
Change-Id: If00f5bc1d1a70396171eddd6a643fbd860e8508c Signed-off-by: Ed Tanous <ed@tanous.net>
show more ...
|
#
1873a04f |
| 01-Apr-2024 |
Myung Bae <myungbae@us.ibm.com> |
Reduce multi-level calls of req.req members
Several places access the members of `req` indirectly like `req.req.method()`. This can be simplified as `req.method()` .
This would also make the code
Reduce multi-level calls of req.req members
Several places access the members of `req` indirectly like `req.req.method()`. This can be simplified as `req.method()` .
This would also make the code clearer.
Tested: - Compiles - Redfish service validator passes
Change-Id: Ie129564ff907cdea7ac224b1e3d80cc0dedfbd7b Signed-off-by: Myung Bae <myungbae@us.ibm.com>
show more ...
|
#
d547d8d2 |
| 16-Mar-2024 |
Ed Tanous <ed@tanous.net> |
Check optionals in tidy
clang-tidy-18 makes this feature stable enough for us to use in general. Enable the check, and fix the couple of regressions that have snuck in since we last ran the check.
Check optionals in tidy
clang-tidy-18 makes this feature stable enough for us to use in general. Enable the check, and fix the couple of regressions that have snuck in since we last ran the check.
Tidy seems to not be able to understand that ASSERT will not continue, so if we ASSERT a std::optional, it's not a bug. Add explicit checks to keep tidy happy.
Tested: clang-tidy passes.
Change-Id: I0986453851da5471056a7b47b8ad57a9801df259 Signed-off-by: Ed Tanous <ed@tanous.net>
show more ...
|
#
47f2934c |
| 19-Mar-2024 |
Ed Tanous <ed@tanous.net> |
Fix redundant init issues
clang-tidy-18 must've fixed their checking for these in headers. Resolve as the robot commands.
Tested: Noop changes made by tidy. Code compiles.
Change-Id: I1de7686c597
Fix redundant init issues
clang-tidy-18 must've fixed their checking for these in headers. Resolve as the robot commands.
Tested: Noop changes made by tidy. Code compiles.
Change-Id: I1de7686c597deffb0df91c30dae1a29f9ba7900e Signed-off-by: Ed Tanous <ed@tanous.net>
show more ...
|
#
a07e9819 |
| 19-Mar-2024 |
Ed Tanous <ed@tanous.net> |
Don't use switch for single conditional http2
This code was copied from one of the nghttp2 examples, and makes things more complicated than they should be. We only handle one case here, so a patter
Don't use switch for single conditional http2
This code was copied from one of the nghttp2 examples, and makes things more complicated than they should be. We only handle one case here, so a pattern of returning early is easier.
Also, this resolves a possible clang-tidy bugprone warning (that we don't yet enable).
Tested: Http2 unit tests pass (good coverage for this case).
Change-Id: Ie8606872f3a96f1bb0329bf22a4f7429f431bbef Signed-off-by: Ed Tanous <ed@tanous.net>
show more ...
|
#
325310d3 |
| 15-Mar-2024 |
Ed Tanous <ed@tanous.net> |
Allow reading http2 bodies
This allows http2 connections to now host authenticated endpoints. Note, this work exposed that the http2 path was not calling preparePayload() and responses were therefor
Allow reading http2 bodies
This allows http2 connections to now host authenticated endpoints. Note, this work exposed that the http2 path was not calling preparePayload() and responses were therefore missing the Content-Length header. preparePayload is now called, and Content-Length is added to the unit tests.
This commit also allows a full Redfish Service Validator test to pass entirely using HTTP2.
Tested: Unit tests pass.
Curl /redfish/v1/Managers/bmc/LogServices/Journal/Entries (which returns a payload larger than 16kB) succeeds and returns the data.
Manually logging in with both basic and session authentication succeeds over http2.
A modified Redfish-Service-Validator, changed to use httpx as its backend, (thus using http2) succeeds.
Change-Id: I956f3ff8f442e9826312c6147d7599ab136a8e7c Signed-off-by: Ed Tanous <ed@tanous.net>
show more ...
|
#
b2896149 |
| 31-Jan-2024 |
Ed Tanous <ed@tanous.net> |
Rename FileBody to HttpBody
Now that our custom body type does things more than files, it makes sense to rename it. This commit renames the header itself, then all instances of the class.
Tested:
Rename FileBody to HttpBody
Now that our custom body type does things more than files, it makes sense to rename it. This commit renames the header itself, then all instances of the class.
Tested: Basic GET requests succeed. Change-Id: If4361ac8992fc7c268f48a336707f96e68d3576c Signed-off-by: Ed Tanous <ed@tanous.net>
show more ...
|
#
d0882189 |
| 27-Jan-2024 |
Ed Tanous <ed@tanous.net> |
Simplify HTTP/2 buffering
Using the mem_send methods of nghttp2 can reduce the amount of buffering we need to do. This is recommended by the nghttp2 docs.
Tested: Enabled experimental-http. Curl
Simplify HTTP/2 buffering
Using the mem_send methods of nghttp2 can reduce the amount of buffering we need to do. This is recommended by the nghttp2 docs.
Tested: Enabled experimental-http. Curl succeeds on /redfish/v1, and shows:
* using HTTP/2 * [HTTP/2] [1] OPENED stream for https://localhost:18080/redfish/v1
Change-Id: I287d8c956f064d244116fac853055a17fca915a2 Signed-off-by: Ed Tanous <ed@tanous.net>
show more ...
|
#
52e31629 |
| 23-Jan-2024 |
Ed Tanous <ed@tanous.net> |
Simplify body
Now that we have a custom boost http body class, we can use it in more cases. There's some significant overhead and code when switching to a file body, namely removing all the headers
Simplify body
Now that we have a custom boost http body class, we can use it in more cases. There's some significant overhead and code when switching to a file body, namely removing all the headers. Making the body class support strings would allow us to completely avoid that inefficiency. At the same time, it would mean that we can now use that class for all cases, including HttpClient, and http::Request. This leads to some code reduction overall, and means we're reliant on fewer beast structures.
As an added benefit, we no longer have to take a dependency on boost::variant2.
Tested: Redfish service validator passes, with the exception of badNamespaceInclude, which is showing warnings prior to this commit.
Change-Id: I061883a73230d6085d951c15891465c2c8445969 Signed-off-by: Ed Tanous <ed@tanous.net>
show more ...
|
#
18f8f608 |
| 18-Jul-2023 |
Ed Tanous <edtanous@google.com> |
Remove some boost includes
The less we rely on boost, and more on std algorithms, the less people have to look up, and the more likely that our code will deduplicate.
Replace all uses of boost::alg
Remove some boost includes
The less we rely on boost, and more on std algorithms, the less people have to look up, and the more likely that our code will deduplicate.
Replace all uses of boost::algorithms with std alternatives.
Tested: Redfish Service Validator passes.
Change-Id: I8a26f39b5709adc444b4178e92f5f3c7b988b05b Signed-off-by: Ed Tanous <edtanous@google.com>
show more ...
|
#
27b0cf90 |
| 07-Aug-2023 |
Ed Tanous <ed@tanous.net> |
Move to file_body in boost
As is, it reads the whole file into memory before sending it. While fairly fast for the user, this wastes ram, and makes bmcweb less useful on less capable systems.
This
Move to file_body in boost
As is, it reads the whole file into memory before sending it. While fairly fast for the user, this wastes ram, and makes bmcweb less useful on less capable systems.
This patch enables using the boost::beast::http::file_body type, which has more efficient serialization semantics than using a std::string. To do this, it adds a openFile() handler to http::Response, which can be used to properly open a file. Once the file is opened, the existing string body is ignored, and the file payload is sent instead. openFile() also returns success or failure, to allow users to properly handle 404s and other errors.
To prove that it works, I moved over every instance of direct use of the body() method over to using this, including the webasset handler. The webasset handler specifically should help with system load when doing an initial page load of the webui.
Tested: Redfish service validator passes.
Change-Id: Ic7ea9ffefdbc81eb985de7edc0fac114822994ad Signed-off-by: Ed Tanous <ed@tanous.net>
show more ...
|
#
5a39f77a |
| 20-Oct-2023 |
Patrick Williams <patrick@stwcx.xyz> |
clang-format: copy latest and re-format
clang-format-17 has some backwards incompatible changes that require additional settings for best compatibility and re-running the formatter. Copy the latest
clang-format: copy latest and re-format
clang-format-17 has some backwards incompatible changes that require additional settings for best compatibility and re-running the formatter. Copy the latest .clang-format from the docs repository and reformat the repository.
Change-Id: I2f9540cf0d545a2da4d6289fc87b754f684bc9a7 Signed-off-by: Patrick Williams <patrick@stwcx.xyz>
show more ...
|
#
f42e8590 |
| 25-Aug-2023 |
Ed Tanous <ed@tanous.net> |
Fix http2 stream pointer
Response and Request are now movable, so lets use that to our advantage and make this no longer require a pointer. This removes a couple NOLINT exceptions in our code, and
Fix http2 stream pointer
Response and Request are now movable, so lets use that to our advantage and make this no longer require a pointer. This removes a couple NOLINT exceptions in our code, and cleans up a lot of places where we could potentially get a nullptr.
Tested: enabled http2-experimental option. Loaded service root from redfish in curl with logging enabled, logging verified http/2 was being used.
Redfish service validator passes. Curl compiled with http returns service root correctly.
Change-Id: I65e11a2311be982df594086413d52838235e1a0c Signed-off-by: Ed Tanous <ed@tanous.net>
show more ...
|
#
e01d0c36 |
| 30-Jun-2023 |
Ed Tanous <edtanous@google.com> |
Fix bugprone-unchecked-optional-access findings
Clang-tidy has the aforementioned check, which shows a few places in the core where we ignored the required optional checks. Fix all uses. Note, we c
Fix bugprone-unchecked-optional-access findings
Clang-tidy has the aforementioned check, which shows a few places in the core where we ignored the required optional checks. Fix all uses. Note, we cannot enable the check that this time because of some weird code in health.hpp that crashes tidy[1]. That will need to be a future improvement.
There are tests that call something like ASSERT(optional) EXPECT(optional->foo())
While this isn't an actual violation, clang-tidy doesn't seem to be smart enough to deal with it, so add some explicit checks.
[1] https://github.com/llvm/llvm-project/issues/55530
Tested: Redfish service validator passes.
Change-Id: Ied579cd0b957efc81aff5d5d1091a740a7a2d7e3 Signed-off-by: Ed Tanous <edtanous@google.com>
show more ...
|
#
62598e31 |
| 17-Jul-2023 |
Ed Tanous <ed@tanous.net> |
Replace logging with std::format
std::format is a much more modern logging solution, and gives us a lot more flexibility, and better compile times when doing logging.
Unfortunately, given its level
Replace logging with std::format
std::format is a much more modern logging solution, and gives us a lot more flexibility, and better compile times when doing logging.
Unfortunately, given its level of compile time checks, it needs to be a method, instead of the stream style logging we had before. This requires a pretty substantial change. Fortunately, this change can be largely automated, via the script included in this commit under scripts/replace_logs.py. This is to aid people in moving their patchsets over to the new form in the short period where old patches will be based on the old logging. The intention is that this script eventually goes away.
The old style logging (stream based) looked like.
BMCWEB_LOG_DEBUG << "Foo " << foo;
The new equivalent of the above would be: BMCWEB_LOG_DEBUG("Foo {}", foo);
In the course of doing this, this also cleans up several ignored linter errors, including macro usage, and array to pointer deconstruction.
Note, This patchset does remove the timestamp from the log message. In practice, this was duplicated between journald and bmcweb, and there's no need for both to exist.
One design decision of note is the addition of logPtr. Because the compiler can't disambiguate between const char* and const MyThing*, it's necessary to add an explicit cast to void*. This is identical to how fmt handled it.
Tested: compiled with logging meson_option enabled, and launched bmcweb
Saw the usual logging, similar to what was present before: ``` [Error include/webassets.hpp:60] Unable to find or open /usr/share/www/ static file hosting disabled [Debug include/persistent_data.hpp:133] Restored Session Timeout: 1800 [Debug redfish-core/include/event_service_manager.hpp:671] Old eventService config not exist [Info src/webserver_main.cpp:59] Starting webserver on port 18080 [Error redfish-core/include/event_service_manager.hpp:1301] inotify_add_watch failed for redfish log file. [Info src/webserver_main.cpp:137] Start Hostname Monitor Service... ``` Signed-off-by: Ed Tanous <ed@tanous.net>
Change-Id: I86a46aa2454be7fe80df608cb7e5573ca4029ec8
show more ...
|
#
fca2cbea |
| 28-Jan-2021 |
Ed Tanous <edtanous@google.com> |
HTTP/2 support
HTTP/2 gives a number of optimizations, while keeping support for the protocol. HTTP/2 support was recently added to the Redfish specification. The largest performance increase in b
HTTP/2 support
HTTP/2 gives a number of optimizations, while keeping support for the protocol. HTTP/2 support was recently added to the Redfish specification. The largest performance increase in bmc usage is likely header compression. Almost all requests reuse the same header values, so the hpack based compression scheme in HTTP/2 allows OpenBMC to be more efficient as a transport, and has the potential to significantly reduce the number of bytes we're sending on the wire.
This commit adds HTTP2 support to bmcweb through nghttp2 library. When static linked into bmcweb, this support adds 53.4KB to the bmcweb binary size. nghttp2 is available in meta-oe already.
Given the experimental nature of this option, it is added under the meson option "experimental-http2" and disabled by default. The hope is to enable it at some point in the future.
To accomplish the above, there a new class, HTTP2Connection is created. This is intended to isolate HTTP/2 connections code from HttpConnection such that it is far less likely to cause bugs, although it does duplicate about 20 lines of code (async_read_some, async_write_some, buffers, etc). This seems worth it for the moment.
In a similar way to Websockets, when an HTTP/2 connection is detected through ALPN, the HTTP2Connection class will be instantiated, and the socket object passed to it, thus allowing the Connection class to be destroyed, and the HTTP2Connection to take over for the user.
Tested: Redfish service validator passes with option enabled With option disabled GET /redfish/v1 in curl shows ALPN non negotiation, and fallback to http1.1
With the option enable GET /redfish/v1 in curl shows ALPN negotiates to HTTP2
Change-Id: I7839e457e0ba918b0695e04babddd0925ed3383c Signed-off-by: Ed Tanous <edtanous@google.com>
show more ...
|