[doc] BFD/SCION protocol specification #3873
Conversation
There was a problem hiding this comment.
Reviewed 3 of 3 files at r1.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @sustrik)
doc/protocols/scion-header.rst, line 58 at r1 (raw file):
The PathType specifies the SCION path type with up to 256 different types. The format of each path type is independent of each other. The initially proposed SCION path types are SCION (0), OneHopPath (1), EPIC (2) and
Can we also change this here? Empty = 0, SCION = 1, OHP = 2, etc ?
There was a problem hiding this comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @lukedirtwalker)
doc/protocols/scion-header.rst, line 58 at r1 (raw file):
Previously, lukedirtwalker (Lukas Vogel) wrote…
Can we also change this here? Empty = 0, SCION = 1, OHP = 2, etc ?
Done.
There was a problem hiding this comment.
Reviewed 2 of 2 files at r2.
Reviewable status:complete! all files reviewed, all discussions resolved
|
doc/protocols/bfd.rst, line 45 at r2 (raw file):
what is the content of the OneHopPath? and how is it being used? |
|
doc/protocols/bfd.rst, line 54 at r2 (raw file):
The source address as it appears in the SCION header is the same address we have when we initialize the router? |
There was a problem hiding this comment.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @karampok, @sgmonroy, and @shitz)
doc/protocols/bfd.rst, line 45 at r2 (raw file):
Previously, karampok (Konstantinos) wrote…
what is the content of the OneHopPath? and how is it being used?
That's part of SCION header specification: https://internal.docs.anapaya.net/en/latest/src/protocols/scion-header.html#path-type-onehoppath
doc/protocols/bfd.rst, line 54 at r2 (raw file):
Previously, karampok (Konstantinos) wrote…
The source address as it appears in the SCION header is the same address we have when we initialize the router?
https://github.com/Anapaya/scion/blob/anapaya/go/pkg/router/connector.go#L63
yes
doc/protocols/bfd.rst, line 54 at r2 (raw file):
Previously, karampok (Konstantinos) wrote…
The source address as it appears in the SCION header is the same address we have when we initialize the router?
yes
There was a problem hiding this comment.
Reviewable status: all files reviewed, 11 unresolved discussions (waiting on @karampok, @sgmonroy, and @sustrik)
doc/protocols/bfd.rst, line 24 at r2 (raw file):
| SCION address header | +----------------------+ | SCION path |
header
doc/protocols/bfd.rst, line 35 at r2 (raw file):
be
by
doc/protocols/bfd.rst, line 45 at r2 (raw file):
Previously, sustrik (Martin Sustrik) wrote…
That's part of SCION header specification: https://internal.docs.anapaya.net/en/latest/src/protocols/scion-header.html#path-type-onehoppath
Maybe add a link to the spec. Also, use https://scion.docs.anapaya.net/...
doc/protocols/bfd.rst, line 48 at r2 (raw file):
was received on.
Theoretically, you could have multiple remote IFIDs per local IFID, thus it's the combination of IFIDs that uniquely identify the session.
doc/protocols/bfd.rst, line 50 at r2 (raw file):
SCION border router
SCION router
doc/protocols/bfd.rst, line 52 at r2 (raw file):
EmptyPath type
Also add a link to the spec.
doc/protocols/bfd.rst, line 58 at r2 (raw file):
border router.
SCION router
doc/protocols/scion-header.rst, line 411 at r2 (raw file):
It has no additional fields.
..., i.e., it consumes 0 bytes on the wire.
doc/protocols/scion-header.rst, line 412 at r2 (raw file):
Keep in mind that address header is still present and must be correctly filled in.
This sentence is superfluous. The path has nothing to do with the address header.
There was a problem hiding this comment.
Reviewable status: all files reviewed, 10 unresolved discussions (waiting on @karampok, @sgmonroy, and @shitz)
doc/protocols/bfd.rst, line 24 at r2 (raw file):
Previously, shitz (Samuel Hitz) wrote…
header
Done.
doc/protocols/bfd.rst, line 35 at r2 (raw file):
Previously, shitz (Samuel Hitz) wrote…
beby
Done.
doc/protocols/bfd.rst, line 48 at r2 (raw file):
Previously, shitz (Samuel Hitz) wrote…
was received on.Theoretically, you could have multiple remote IFIDs per local IFID, thus it's the combination of IFIDs that uniquely identify the session.
There's only one remote IFID, as discussed in person.
doc/protocols/bfd.rst, line 50 at r2 (raw file):
Previously, shitz (Samuel Hitz) wrote…
SCION border routerSCION router
Done.
doc/protocols/bfd.rst, line 52 at r2 (raw file):
Previously, shitz (Samuel Hitz) wrote…
EmptyPath typeAlso add a link to the spec.
Done.
doc/protocols/bfd.rst, line 58 at r2 (raw file):
Previously, shitz (Samuel Hitz) wrote…
border router.SCION router
Done.
doc/protocols/scion-header.rst, line 411 at r2 (raw file):
Previously, shitz (Samuel Hitz) wrote…
It has no additional fields...., i.e., it consumes 0 bytes on the wire.
Done.
doc/protocols/scion-header.rst, line 412 at r2 (raw file):
Previously, shitz (Samuel Hitz) wrote…
Keep in mind that address header is still present and must be correctly filled in.This sentence is superfluous. The path has nothing to do with the address header.
Done.
There was a problem hiding this comment.
Reviewed 2 of 2 files at r3.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @karampok, @sgmonroy, and @sustrik)
doc/protocols/bfd.rst, line 40 at r3 (raw file):
Bootstrapping in SCION Router
Somewhere you should mention that Your Discrimantor (or My Discriminator in the reply packet) is chosen randomly.
There was a problem hiding this comment.
Reviewable status: 2 of 3 files reviewed, 3 unresolved discussions (waiting on @karampok, @sgmonroy, and @shitz)
doc/protocols/bfd.rst, line 40 at r3 (raw file):
Previously, shitz (Samuel Hitz) wrote…
Bootstrapping in SCION RouterSomewhere you should mention that
Your Discrimantor(orMy Discriminatorin the reply packet) is chosen randomly.
Done.
There was a problem hiding this comment.
Reviewed 1 of 1 files at r4.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @sgmonroy and @shitz)
There was a problem hiding this comment.
Reviewable status:
This change is