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

Network socket changes #136

Merged
merged 13 commits into from Mar 7, 2021
Merged

Network socket changes #136

merged 13 commits into from Mar 7, 2021

Conversation

rw-r-r-0644
Copy link
Contributor

Opening this PR to keep track of wutsocket changes/issues/suggestions.

The wutsocket library provides standard network headers and a devoptab for the nsysnet socket library.

Status:

  • Currently the wutsocket library appears to work, though it was only tested with a limited number of applications and more testing is probably required.

Potential issues:

  • The wutsocket library is currently initialized from wutcrt, and internally handles nn_ac and the nsysnet socket library initialization. This is not ideal as some applications might not need to use the network, and the AC initialization might require some time (this last issue can partially be resolved using async initialization).

  • Also the nsysnet retrocompatibility layer, while present, could potentially cause issues with a limited number of applications (for example apps manually defining nsysnet error codes, which won't match the returned errno codes).

Any suggestion regarding those or other possible issues is welcome

@ashquarky
Copy link
Contributor

ACConnectWithConfigIdAsync does exist, though it's not in Decaf or wut's headers atm. May need some light reverse-engineering work, with any luck it's just passing a callback through to IoctlAsync with no extra weirdness on top.

Copy link
Contributor

@ashquarky ashquarky left a comment

Choose a reason for hiding this comment

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

Looks pretty good, all up! I'm not convinced about having all the socket headers in include/ and include/sys/. Did you write them yourself? Might be a little more maintainable to nick 'em from BSD, like what libnx does (they also bury it all under external/). Just minor nitpicks, though!

include/nsysnet/_socket.h Outdated Show resolved Hide resolved
@@ -2,11 +2,13 @@ void __init_wut_malloc();
void __init_wut_newlib();
extern void __init_wut_stdcpp() __attribute__((weak));
void __init_wut_devoptab();
void __init_wut_socket();
Copy link
Contributor

Choose a reason for hiding this comment

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

These could potentially be weak functions? Could be one way around the AC issue

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Definitively an option; waiting for more comments on this

libraries/wutsocket/wut_socket.c Outdated Show resolved Hide resolved
libraries/wutsocket/wut_socket_common.c Show resolved Hide resolved
libraries/wutsocket/wut_socket_common.c Show resolved Hide resolved
@rw-r-r-0644
Copy link
Contributor Author

Looks pretty good, all up! I'm not convinced about having all the socket headers in include/ and include/sys/. Did you write them yourself? Might be a little more maintainable to nick 'em from BSD, like what libnx does (they also bury it all under external/). Just minor nitpicks, though!

Thanks for the suggestions! 👍

Not sure about the bsd socket headers; while they are what libnx uses, other platforms, such as libctru, also provide their own headers. They work very well for the Switch since the os provides an almost complete bsd sockets implementation, but on the wiiu many of the additional symbols provided by those headers are not available/wouldn't have any effect, so they might result in unnecessary clutter. Also as nintendo won't update nsysnet anymore, we probably wouldn't really need that much maintainance for the headers after they are working

@rw-r-r-0644
Copy link
Contributor Author

Just realized we might have one additional issue related to these changes: libcurl allows applications to interact with the network sockets used by the library, which for nintendo's libcurl port would be coming from the nsysnet library rather than newlib.
It could make sense to replace nintendo's libcurl altogether and provide a libcurl and mbedtls packages as with the switch or the 3ds. The builtin libcurl port is outdated, uses NSSL for https, and won't work with anything newer than TLSv1.0 (so most https websites are currently not working)

@ashquarky
Copy link
Contributor

Not sure about the bsd socket headers; while they are what libnx uses, other platforms, such as libctru, also provide their own headers. They work very well for the Switch since the os provides an almost complete bsd sockets implementation, but on the wiiu many of the additional symbols provided by those headers are not available/wouldn't have any effect, so they might result in unnecessary clutter. Also as nintendo won't update nsysnet anymore, we probably wouldn't really need that much maintainance for the headers after they are working

This is a fair point! Just wanted to make sure we're following standards here. POSIX all the way! Maybe there's some kind of test suite we could run these headers against? Maybe I'm just nervous, I dunno.

Just realized we might have one additional issue related to these changes: libcurl allows applications to interact with the network sockets used by the library, which for nintendo's libcurl port would be coming from the nsysnet library rather than newlib.
It could make sense to replace nintendo's libcurl altogether and provide a libcurl and mbedtls packages as with the switch or the 3ds. The builtin libcurl port is outdated, uses NSSL for https, and won't work with anything newer than TLSv1.0 (so most https websites are currently not working)

I agree that a new libcurl is a better long-term solution, but mbedtls is kinda annoying (I've tried messing with it before, see yawut/mbedtls) so colour me unconvinced we'll be able to pull it off quickly. As a shorter-term solution, wutsocket could rplwrap the relevant libcurl functions too? I'd imagine the actual code would be super trivial, just looking for the conditions where libcurl gives an fd and passing it through the appropriate lookup tables.

@exjam
Copy link
Contributor

exjam commented May 28, 2020

Yes both some curl methods and also NSSLCreateConnection takes a socket file descriptor.

@BullyWiiPlaza
Copy link
Contributor

BullyWiiPlaza commented Nov 10, 2020

It could make sense to replace nintendo's libcurl altogether and provide a libcurl and mbedtls packages as with the switch or the 3ds. The builtin libcurl port is outdated, uses NSSL for https, and won't work with anything newer than TLSv1.0 (so most https websites are currently not working)

Yeah. Any progress on updating the HTTPS support for libcurl (using NSSL or something else) to work seemlessly from a developer's point of view like when the same code is compiled for Windows/Linux? I tried to use it on Wii U and it didn't work as you said but it would be great if that was possible. Only being able to connect to HTTP websites is almost useless. After all, Wii U games are communicating over SSL just fine so the code is definitely there already.

@fincs fincs changed the base branch from master to socket March 7, 2021 12:08
@fincs fincs marked this pull request as ready for review March 7, 2021 12:08
@fincs fincs merged commit c377cff into devkitPro:socket Mar 7, 2021
@fincs
Copy link
Member

fincs commented Mar 7, 2021

I'm going to have a go at finishing this PR.

@GaryOderNichts
Copy link
Contributor

GaryOderNichts commented Mar 7, 2021

I added a few more functions in my fork here: https://github.com/GaryOderNichts/wut/tree/wutsocket
Might be useful for finishing this PR.
These changes are enough to build the latest version of curl, which is why I removed the system curl.

@fincs
Copy link
Member

fincs commented Mar 7, 2021

Good to know. I'm first going to try implementing the weak linking mechanism suggested in one of the conversations above, though. In the mean time, can you PR some of your other fixes? I saw a gettod fix that is unrelated to socket stuff in your branch.

@GaryOderNichts
Copy link
Contributor

Yeah, will do.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants