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.

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

Add ping spec #473

Merged
merged 4 commits into from
Nov 15, 2022
Merged

Add ping spec #473

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8383370
Add ping spec
MarcoPolo Nov 5, 2022
8af5686
More precise language
MarcoPolo Nov 5, 2022
a263550
Fix double space
MarcoPolo Nov 5, 2022
bdbb9a1
Update ping/ping.md
MarcoPolo Nov 15, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ security, multiplexing, and other purposes.

The protocols described below all use [protocol buffers](https://developers.google.com/protocol-buffers/docs/proto?hl=en) (aka protobuf) to define message schemas. Version `proto2` is used unless stated otherwise.

- [ping][spec_ping] - Ping protocol
- [autonat][spec_autonat] - NAT detection
- [identify][spec_identify] - Exchange keys and addresses with other peers
- [kademlia][spec_kademlia] - The Kademlia Distributed Hash Table (DHT) subsystem
Expand Down Expand Up @@ -129,3 +130,4 @@ you feel an issue isn't the appropriate place for your topic, please join our
[spec_autonat]: ./autonat/README.md
[spec_dcutr]: ./relay/DCUtR.md
[spec_webtransport]: ./webtransport/README.md
[spec_ping]: ./ping/ping.md
62 changes: 62 additions & 0 deletions ping/ping.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
# Ping <!-- omit in toc -->

| Lifecycle Stage | Maturity | Status | Latest Revision |
| --------------- | ------------------------ | ------ | --------------- |
| 1A | Candidate Recommendation | Active | r0, 2022-11-04 |

Authors: [@marcopolo]

Interest Group: [@marcopolo], [@mxinden], [@marten-seemann]

[@marcopolo]: https://github.com/mxinden
[@mxinden]: https://github.com/mxinden
[@marten-seemann]: https://github.com/marten-seemann

# Table of Contents <!-- omit in toc -->
- [Protocol](#protocol)
- [Diagram](#diagram)

# Protocol

The ping protocol is a simple request response protocol. The client opens a
stream, sends a payload of 32 random bytes, and the server responds with the
same 32 bytes on that stream. The client then measures the RTT from the time it
wrote the bytes to the time it received the bytes. The client MAY repeat the
process by sending another payload with random bytes on the same stream, so the
server SHOULD loop and echo the next payload. The client SHOULD close the write
side of the stream after sending the last payload, and the server SHOULD finish
Comment on lines +26 to +27
Copy link
Member

@tomaka tomaka Dec 29, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Why is it "SHOULD" rather than "MUST"?

If the server is allowed to not respond to pings (which is implied by the usage of "SHOULD"), then the protocol is pretty useless.

Of course, the client should be aware of the fact that the server can in practice do something else than this text says, but this is off-topic for a specification. The word "MUST" implicitly means "MUST ... in order to comply to this specification".
The possibility that you might be connected to a peer that doesn't conform to the specification doesn't need to be taken into account in the context of the specification itself, otherwise you've got a snake biting its tail.

writing the echoed payload and then exit the loop and close the stream.

The client MUST NOT keep more than one outbound stream for the ping protocol per
peer. The server SHOULD accept at most 2 streams per peer since cross stream
behavior is not linearizable for client and server. In other words, the client
closing stream A and then opening stream B, might be perceived by the server as
the client opening stream B and then closing stream A.
Copy link
Contributor

@Menduist Menduist Nov 5, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

So for instance

The server SHOULD wait 200 milliseconds after a ping before accepting another ping a stream.
The client MUST wait 400 milliseconds after a reply to ping the server again on a stream.

(or make it global instead of per stream, idk)

200ms of margin to allow for jitter at the networking & scheduler level. This is less elegant than just one request per stream, but what can we do. If your ping suddently goes from 500s to 100ms (for instance if the cpu is clogged up), you will have an erroneous value for one ping.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why? This isn’t DOS mitigation. If you’re worried about rate limiting a 32byte response, then you probably want to do something at a higher level (rate limiting all streams and connections for example).


The protocol id is `/ipfs/ping/1.0.0`.

# Diagram

![Ping Protocol Diagram](./ping.svg)

<details>
<summary>Instructions to reproduce diagram</summary>

From the root, run: `plantuml -tsvg ping/ping.md`

```
@startuml
skinparam backgroundColor white

entity Client
entity Server

== /ipfs/ping/1.0.0 ==
loop until Client closes write
Client -> Server: 32 random bytes
Client <- Server: Same 32 random bytes
end
@enduml
```

</details>
22 changes: 22 additions & 0 deletions ping/ping.svg
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.