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

add tvu port binding info to docs #4948

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

Conversation

alexpyattaev
Copy link

Summary of Changes

  • Documenting how we get multiple bound TVU sockets published via gossip.

@alexpyattaev alexpyattaev added the noCI Suppress CI on this Pull Request label Feb 12, 2025
@alexpyattaev alexpyattaev removed the noCI Suppress CI on this Pull Request label Feb 12, 2025
@alexpyattaev
Copy link
Author

@gregcusack do you like this?

@alexpyattaev alexpyattaev marked this pull request as ready for review February 12, 2025 12:35
@alexpyattaev alexpyattaev added the CI Pull Request is ready to enter CI label Feb 12, 2025
@anza-team anza-team removed the CI Pull Request is ready to enter CI label Feb 12, 2025
Copy link

@gregcusack gregcusack 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 putting this together! just a few thoughts. biggest thing is that we should probably differentiate TVU quic and udp sockets. And then somewhere add that TPU is setup somewhat similarly. Although TPU also uses bind_more_with_config which I believe just enables more connections and ability to distribute load better.

docs/src/validator/tvu.md Outdated Show resolved Hide resolved
docs/src/validator/tvu.md Outdated Show resolved Hide resolved
docs/src/validator/tvu.md Outdated Show resolved Hide resolved
docs/src/validator/tvu.md Show resolved Hide resolved

## TVU sockets

Externally, TVU appears to bind to one port, typically 8002 UDP. Internally, TVU is actually bound with multiple sockets

Choose a reason for hiding this comment

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

so we have both UDP and QUIC ports for TVU. Probably makes sense to differentiate these two somewhere. UDP multibind with reuseport works on OS level. For quic tvu, we only bind to one socket. and the multiplexing is done at the protocol level.

Copy link
Author

Choose a reason for hiding this comment

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

done

## TVU sockets

Externally, TVU appears to bind to one port, typically 8002 UDP. Internally, TVU is actually bound with multiple sockets
to improve kernel's handling of the packet queues. It is setup so that a node can advertise one external ip/port for TVU/TPU. We're binding multiple sockets to the same port using SO_REUSEPORT:

Choose a reason for hiding this comment

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

external ip/port for TVU/TPU.

would maybe leave out TPU inline here. Maybe we can add something right after or even at the top saying TPU runs on similar port management logic

Copy link
Author

Choose a reason for hiding this comment

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

added note about TPU

docs/src/validator/tvu.md Show resolved Hide resolved
docs/src/validator/tvu.md Outdated Show resolved Hide resolved
docs/src/validator/tvu.md Outdated Show resolved Hide resolved
docs/src/validator/tvu.md Outdated Show resolved Hide resolved
docs/src/validator/tvu.md Outdated Show resolved Hide resolved
docs/src/validator/tvu.md Outdated Show resolved Hide resolved
docs/src/validator/tvu.md Outdated Show resolved Hide resolved
Comment on lines 55 to 58
```rust
const SOCKET_TAG_TVU: u8 = 10; // For UDP
const SOCKET_TAG_TVU_QUIC: u8 = 11; // For QUIC

Copy link

@gregcusack gregcusack Feb 12, 2025

Choose a reason for hiding this comment

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

Suggested change
```rust
const SOCKET_TAG_TVU: u8 = 10; // For UDP
const SOCKET_TAG_TVU_QUIC: u8 = 11; // For QUIC
```rust
const SOCKET_TAG_TVU: u8 = 10; // For UDP
const SOCKET_TAG_TVU_QUIC: u8 = 11; // For QUIC
^ note the missing code block closing "```"

Choose a reason for hiding this comment

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

ok this formatted weird. but basically, we're missing a ``` at the end of this block, so formatting is weird

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