Skip to content

net: http: service: enhance HTTP service with optional custom socket create #92732

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

net: http: service: enhance HTTP service with optional custom socket create #92732

wants to merge 1 commit into from
+111 −27

Conversation

AndreyDodonov-EH
Copy link
Contributor

Currently webserver does not support custom socket creation e.g. for offloaded socket.

Update the HTTP service API to allow custom socket creation through a function pointer in the service descriptor.
The existing socket creation method is retained as a fallback if no custom function is provided.

This functionality is per-service, i.e. it allows to specify which http services will work on which sockets.
Backwards-compatible.

Partially related to #89960

@github-actions github-actions bot added area: HTTP HTTP client/server support area: Networking labels Jul 5, 2025
@AndreyDodonov-EH AndreyDodonov-EH mentioned this pull request Jul 5, 2025
Open
@AndreyDodonov-EH AndreyDodonov-EH force-pushed the feature/custom-socket-init-in-webserver branch from 1c3c8f2 to 6e11e8d Compare July 5, 2025 17:36
@AndreyDodonov-EH AndreyDodonov-EH changed the title net: http: srvice: enhance HTTP service API with optional custom socket creation net: http: srvice: enhance HTTP service with optional custom socket init Jul 5, 2025
@AndreyDodonov-EH AndreyDodonov-EH force-pushed the feature/custom-socket-init-in-webserver branch 2 times, most recently from 84fe49b to 3aadf05 Compare July 5, 2025 18:08
@AndreyDodonov-EH AndreyDodonov-EH changed the title net: http: srvice: enhance HTTP service with optional custom socket init net: http: srvice: enhance HTTP service with optional custom socket create Jul 5, 2025
@AndreyDodonov-EH AndreyDodonov-EH force-pushed the feature/custom-socket-init-in-webserver branch from 3aadf05 to 5770cd8 Compare July 5, 2025 18:19
rlubos
rlubos reviewed Jul 7, 2025
View reviewed changes
include/zephyr/net/http/service.h Outdated
@@ -220,14 +225,14 @@ struct http_service_desc {
* @param _sec_tag_list_size TLS security tag list size used to setup a HTTPS socket.
*/
#define HTTPS_SERVICE_DEFINE(_name, _host, _port, _concurrent, _backlog, _detail, \
_res_fallback, _sec_tag_list, _sec_tag_list_size) \
_res_fallback, _sec_tag_list, _sec_tag_list_size, ...) \
Copy link
Collaborator

Choose a reason for hiding this comment

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

TBH I think I'd prefer to keep all of the macro parameters explicit instead of hiding optional parameters behind __VA_ARGS__ - it might get super confusing if someone wanted to add yet another parameter in the future, it'd be tricky to document also.

We could just extend the macros/service struct with an additional struct http_service_config where we could keep any optional per-service configurations, including this custom socket struct, so in case any other custom configurations will be needed in the future, we could just extend the struct w/o touching the macros again.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@rlubos
Good point!
If we want to both prevent future modifications and stay backwards compatible,
I can also alternatively create new macroses which accept struct http_service_config

Copy link
Collaborator

Choose a reason for hiding this comment

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

Well, the API is still experimental so I'm not sure if we need to care that much about backward comptibility, I'd just update the existing macros.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@rlubos
I modified the API as you suggested.
Could you please have a brief look?

If that change is okay for you, I'll go ahead with the formalities - fixing samples, docs, twister tests etc. (never done twister tests in Zephyr itself yet)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jukkar @mrodgers-witekio
Could you please have a look too? I don't want to move ahead with fixing the tests and samples if the change is not okay for you

Copy link
Collaborator

Choose a reason for hiding this comment

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

The approach looks good to me.

May be worth adding a note in the migration guide to mention the change to the public API, although I've just noticed that there isn't a draft migration guide for v4.2 --> v4.3 yet (assuming this change won't make it into the v4.2 release), anyone know who would usually create this?

@AndreyDodonov-EH AndreyDodonov-EH force-pushed the feature/custom-socket-init-in-webserver branch 2 times, most recently from 6cf7a0b to 2f72b82 Compare July 9, 2025 15:54
@AndreyDodonov-EH AndreyDodonov-EH mentioned this pull request Jul 9, 2025
Open
rlubos
rlubos reviewed Jul 10, 2025
View reviewed changes
Copy link
Collaborator

@rlubos rlubos left a comment

Choose a reason for hiding this comment

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

Looks good, IMHO we can proceed with this approach.

subsys/net/lib/http/http_server_core.c Outdated
@@ -156,7 +156,11 @@ int http_server_init(struct http_server_ctx *ctx)
proto = IPPROTO_TCP;
}

fd = zsock_socket(af, SOCK_STREAM, proto);
if (svc->config != NULL && svc->config->socket_creat != NULL) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Typo: socket_creat

@AndreyDodonov-EH AndreyDodonov-EH changed the title net: http: srvice: enhance HTTP service with optional custom socket create net: http: service: enhance HTTP service with optional custom socket create Jul 10, 2025
@AndreyDodonov-EH AndreyDodonov-EH force-pushed the feature/custom-socket-init-in-webserver branch 2 times, most recently from a0dbf21 to ba165ca Compare July 10, 2025 17:12
@github-actions github-actions bot added area: Samples Samples area: Shell Shell subsystem area: Sockets Networking sockets Release Notes To be mentioned in the release notes labels Jul 10, 2025
@AndreyDodonov-EH AndreyDodonov-EH force-pushed the feature/custom-socket-init-in-webserver branch 2 times, most recently from 122e2df to 405ce47 Compare July 10, 2025 17:52
@AndreyDodonov-EH AndreyDodonov-EH force-pushed the feature/custom-socket-init-in-webserver branch from 405ce47 to 0f8b7b7 Compare July 10, 2025 18:20
@AndreyDodonov-EH
Copy link
Contributor Author

Updated tests and docs.

Also updated release notes, not sure if it is needed for 4.2

@kartben
Copy link
Collaborator

kartben commented Jul 10, 2025

Updated tests and docs.

Also updated release notes, not sure if it is needed for 4.2

Please update 4.3 release notes instead given this won't make it into 4.2 which is currently in feature freeze and a week away from being released. Thanks!

@AndreyDodonov-EH
net: http: service: extend HTTP service with config
0dc2add 
Update the HTTP service API to allow setting per-service configuration,
currently it is only custom socket creation callback,
but in the future it can be extended with other parameter

Signed-off-by: Andrey Dodonov <Andrey.Dodonov@endress.com>
@AndreyDodonov-EH AndreyDodonov-EH force-pushed the feature/custom-socket-init-in-webserver branch from e164b8b to 0dc2add Compare July 11, 2025 06:44
@sonarqubecloud SonarQubeCloud
Copy link

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

rlubos
rlubos approved these changes Jul 11, 2025
View reviewed changes
Copy link
Collaborator

@rlubos rlubos left a comment

Choose a reason for hiding this comment

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

Looks good, thanks!

@dkalowsk dkalowsk added this to the v4.3.0 milestone Jul 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jukkar jukkar jukkar approved these changes

@rlubos rlubos rlubos approved these changes

@mrodgers-witekio mrodgers-witekio Awaiting requested review from mrodgers-witekio

@carlescufi carlescufi Awaiting requested review from carlescufi

@danieldegrasse danieldegrasse Awaiting requested review from danieldegrasse

@dkalowsk dkalowsk Awaiting requested review from dkalowsk

@jakub-uC jakub-uC Awaiting requested review from jakub-uC

@kartben kartben Awaiting requested review from kartben

@nashif nashif Awaiting requested review from nashif

@pdgendt pdgendt Awaiting requested review from pdgendt

@ssharks ssharks Awaiting requested review from ssharks

Assignees

@jukkar jukkar

@rlubos rlubos

Labels
area: HTTP HTTP client/server support area: Networking area: Samples Samples area: Shell Shell subsystem area: Sockets Networking sockets Release Notes To be mentioned in the release notes
Projects
None yet
Milestone
v4.3.0
Development

Successfully merging this pull request may close these issues.
6 participants
@AndreyDodonov-EH @kartben @jukkar @rlubos @mrodgers-witekio @dkalowsk