Skip to content

Stubs for UDP socket options SO_BROADCAST, SO_REUSEADDR, SO_REUSEPORT#3434

Draft
Nashatyrev wants to merge 1 commit intoshadow:mainfrom
Nashatyrev:teku-stubs
Draft

Stubs for UDP socket options SO_BROADCAST, SO_REUSEADDR, SO_REUSEPORT#3434
Nashatyrev wants to merge 1 commit intoshadow:mainfrom
Nashatyrev:teku-stubs

Conversation

@Nashatyrev
Copy link
Copy Markdown

@Nashatyrev Nashatyrev commented Nov 6, 2024

I've managed to run Ethereum Java client Teku inside Shadow (specifically with Ethshadow).
However I had to add the following stubs to make it working finally:

  • UDP SO_BROADCAST: Netty library (widely adopted jvm network lib) asks for this socket option prior to binding a UDP port. No workarounds on a client code side were found for this.
  • UDP SO_REUSEADDR & SO_REUSEPORT: the option is set in the Teku app itself to check if the specified port is not busy and fail fast if busy. Not sure if it's that necessary here, but it makes sense because this port is then promptly closed and reopened again (when actual port binding occurs). This may potentially fail due to socket close delay in OS (probably it may be the case just in some specific OS)

To be honest I'm not quite sure if it's the right thing to merge these stubs as is (relates to #3280), so kindly asking for devs assistance.

@github-actions github-actions bot added Component: Testing Unit and integration tests and frameworks Component: Main Composing the core Shadow executable labels Nov 6, 2024
@stevenengler
Copy link
Copy Markdown
Contributor

stevenengler commented Nov 7, 2024

Thanks for the detailed notes on these issues! Yeah, we still don't have a good way of dealing with stubs. We experimented in #3332, #3369, and #3375 with adding experimental options to support stubs, but we weren't happy with any of those approaches. If you have alternative ideas, feel free to mention on #3280. But I think for now our plan is to not merge any additional stubs unless they're "correct".

I think if you make a separate PR with just the "return 0 for SO_BROADCAST in getsockopt", we could merge that one. We'd need a test for it in src/test/socket/sockopt/test_sockopt.rs (verify that getsockopt(SO_BROADCAST) returns 0 for new sockets on Linux and Shadow). Depending on the tests, it might also make sense to return 0 for tcp and unix stream/dgram/seqpacket sockets as well.

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

Labels

Component: Main Composing the core Shadow executable Component: Testing Unit and integration tests and frameworks

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants