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

Probing packet #1253

Merged
merged 3 commits into from
Mar 27, 2018
Merged

Probing packet #1253

merged 3 commits into from
Mar 27, 2018

Conversation

janaiyengar
Copy link
Contributor

Define probing/non-probing packet and frames.

is a "probing packet", and a packet containing any other frame is a "non-probing
packet".

An endpoint MUST NOT send non-probing frames to a peer address until it receives
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this statement true? What about NAT rebinds?

Copy link
Contributor

Choose a reason for hiding this comment

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

If this statement is strictly followed, no one ever migrates because they're waiting for the other side to do so. (At which the other side has violated a MUST, and you really ought to kill the connection to that naughty endpoint!)

For both client and server migration, this actually points in the same direction, so let's accept the non-generic here: "A server MUST NOT send non-probing packets to a client address until it receives a non-probing packet from that address."

Copy link
Contributor

@MikeBishop MikeBishop left a comment

Choose a reason for hiding this comment

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

(mistaken empty review which I can't delete)

Copy link
Contributor

@MikeBishop MikeBishop left a comment

Choose a reason for hiding this comment

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

Good on the targeted fix, but let's get a drive-by fix to the adjacent text while we're here.

is a "probing packet", and a packet containing any other frame is a "non-probing
packet".

An endpoint MUST NOT send non-probing frames to a peer address until it receives
Copy link
Contributor

Choose a reason for hiding this comment

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

If this statement is strictly followed, no one ever migrates because they're waiting for the other side to do so. (At which the other side has violated a MUST, and you really ought to kill the connection to that naughty endpoint!)

For both client and server migration, this actually points in the same direction, so let's accept the non-generic here: "A server MUST NOT send non-probing packets to a client address until it receives a non-probing packet from that address."

@janaiyengar janaiyengar merged commit acd7db8 into master Mar 27, 2018
martinthomson added a commit that referenced this pull request Mar 28, 2018
This paragraph:

> A server MUST NOT send non-probing frames to a client's address until the server
> receives a non-probing packet from that address.

Is true only with the restriction of migration to client addresses.  It
complicates the server migration story if we ever want to do that (as we
already have with the proposal for preferred addresses).
martinthomson added a commit that referenced this pull request Apr 4, 2018
This paragraph:

> A server MUST NOT send non-probing frames to a client's address until the server
> receives a non-probing packet from that address.

Is true only with the restriction of migration to client addresses.  It
complicates the server migration story if we ever want to do that (as we
already have with the proposal for preferred addresses).
martinthomson added a commit that referenced this pull request Apr 5, 2018
Remove unnecessary text from #1253
@MikeBishop MikeBishop deleted the probing branch June 14, 2018 23:41
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