daemon/snet/showpaths: allow zero for hop latency #4005
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
lukedirtwalker
requested changes
Apr 7, 2021
There was a problem hiding this comment.
Reviewed 3 of 3 files at r1.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @matzf)
go/lib/infra/modules/combinator/staticinfo_accumulator.go, line 110 at r1 (raw file):
latencies[i] = l } else { latencies[i] = snet.LatencyUnset
this could be set unconditionally before, then no else is needed.
go/lib/infra/modules/combinator/staticinfo_accumulator.go, line 119 at r1 (raw file):
return } if v == 0 {
could the passed value be negative?
go/lib/infra/modules/combinator/staticinfo_accumulator.go, line 170 at r1 (raw file):
return } if v == 0 {
bandwidth 0 doesn't make sense to me, what is the use case here?
matzf
commented
Apr 7, 2021
There was a problem hiding this comment.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @lukedirtwalker)
go/lib/infra/modules/combinator/staticinfo_accumulator.go, line 110 at r1 (raw file):
Previously, lukedirtwalker (Lukas Vogel) wrote…
this could be set unconditionally before, then no else is needed.
Done, sort of. Better?
go/lib/infra/modules/combinator/staticinfo_accumulator.go, line 119 at r1 (raw file):
Previously, lukedirtwalker (Lukas Vogel) wrote…
could the passed value be negative?
Great point, yes I think it could be negative, as there is nothing up to this point that would filter out such invalid values .
go/lib/infra/modules/combinator/staticinfo_accumulator.go, line 170 at r1 (raw file):
Previously, lukedirtwalker (Lukas Vogel) wrote…
bandwidth 0 doesn't make sense to me, what is the use case here?
Great point, thanks!
I removed it because I thought it was redundant -- thinking it through again, I realized it's not entirely redundant as it will filter out an explicitly announced 0 value in the case that there is conflicting information.
Added the check back.
54e996a to
9ae62f2
Compare
lukedirtwalker
approved these changes
Apr 7, 2021
There was a problem hiding this comment.
Reviewed 1 of 1 files at r2.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @matzf)
Allow to use zero for the latency metadata of each hop. Zero can be a sensible value for this, for example when interfaces are part of the same router instance. Use negative value (-1 nanosecond) to represent undefined values instead.
9ae62f2 to
78e2557
Compare
juagargi
pushed a commit
to juagargi/scion
that referenced
this pull request
Apr 15, 2021
Allow to use zero for the latency metadata of each hop. Zero can be a sensible value for this; for example when multiple interfaces are part of the same router instance, then it makes sense to announce zero for the latency between these interfaces. Use negative value (-1 nanosecond) to represent undefined values internally, instead. Closes scionproto#4005 GitOrigin-RevId: 51357848a53f5ff7c196c1b426f93084ece3f2cf
juagargi
pushed a commit
to netsec-ethz/scion
that referenced
this pull request
Apr 22, 2021
Allow to use zero for the latency metadata of each hop. Zero can be a sensible value for this; for example when multiple interfaces are part of the same router instance, then it makes sense to announce zero for the latency between these interfaces. Use negative value (-1 nanosecond) to represent undefined values internally, instead. Closes scionproto#4005 GitOrigin-RevId: 51357848a53f5ff7c196c1b426f93084ece3f2cf
juagargi
added a commit
to netsec-ethz/scion
that referenced
this pull request
Apr 22, 2021
* scion-pki: send CMS signed renewal request
Add the functionality to send CMS signed renewal requests to scion-pki.
By default, the renewal request contains both the CMS signed, and the
legacy signed request.
Either of the requests can be disabled with a feature flag.
GitOrigin-RevId: b0b073d5559b0cde6f9f3b5b5ee2bf6aac5c72ab
* fix minor issue with former rawbytes field
* ca: add delegating handler
Add a delegating handler that is capable of talking to the CA service
for certificate renewal.
This will be plugged in to `renewal/grpc:RenewalServer` based on a config
option in the control service in a follow-up PR.
Currently, the metrics result label values are not set properly
because there is ongoing work in refactoring the metrics for the
renewal package. This will be amended in a follow-up PR.
GitOrigin-RevId: 49ffd3a6d51a06320a927c4dccf50405917ea923
* AT: collect coredumps
If a process crashes inside a container, the coredump is stored within the CI artifact, in the logs directory. The name of the file is container-name.coredump
GitOrigin-RevId: 5578056189dbd866d3e53f6d881c48a8300f3081
* ping: add --healthy-only flag
Add `--healthy-only` flag to `scion ping`. When the --healthy-only option is set, ping first determines healthy paths through probing and chooses amongst them.
GitOrigin-RevId: 954892aeafa993df8614285cad53cb01ff822fb3
* oai: ‘/certificates/{chain-id}’ and ‘/certificates/{chain-id}/blob'
Add the ‘/certificates/{chain-id}’ and ‘/certificates/{chain-id}/blob’ endpoints which provide the certificate chain (blob) for a given chain-id. In addition to the modification of the spec files, the ‘TrsutAPI’ interface is extended to include a function called ‘Chain’.
GitOrigin-RevId: 6c6c7bc3efe50ab51d59e6f3c2cb5f65720446d6
* scion.sh: use 'make lint' instead of './scion.sh lint'
Let us use 'make lint' instead of ‘./scion.sh lint’.
GitOrigin-RevId: 6dca0805e2954edabf9ad02e4735540fd9749a93
* fake-gateway: add dataplane path configuration
Add `forwarding_path` key s.t. the fake-gatway can consume a provided base64 encoded raw dataplane path.
GitOrigin-RevId: f9c1a4657456464c4b928c0f6f3dda763ed5f369
* epic-hp: controlplane
This PR implements the EPIC control plane changes as described in [1]. It improves the structure of the previous PR [2] according to the suggestion from [3].
- Add an EPICDetachedExtension to the beacon. This extension does not get signed and can therefore be detached.
- Add a DigestExtension to the PathSegmentExtensions, which holds the hash of EPICDetachedExtension. This hash is therefore signed.
- Overhead: no new MAC needs to be calculated, only one additional hash per AS.
[1] https://scion.docs.anapaya.net/en/latest/EPIC.html#control-plane
[2] scionproto#3944
[3] scionproto@3a5ea8e
Closes scionproto#3954
GitOrigin-RevId: 44aea772d6021366b7dc8da5de8d7cc1a7b1ad61
* trust: check queries before passing to database
In scionproto/scion/commit/0ebd1f3690d65247cad9b1ac28f016d83e039b1b,
the semantics of `trust.DB.Chains` was slightly modified to allow
fetching certificate chains for the management API.
This change ensures that the verifier always requests certificate chains
with a fully qualified certificate chain query.
GitOrigin-RevId: 1f99f7c8eacaf9c7db46732216c47e270401e804
* prober: use local endpoint per different underlay network
The path prober is now using a local endpoint per different underlay network. Previously, it was using a single static local endpoint which is not correct in cases where different underlay networks are used to reach different next-hop routers.
GitOrigin-RevId: fd962b0fc24b820216bef9664a2f2c3dfc93ebfc
* ca: reduce duplicated code
Define the `ExtractChain` function in `go/pkg/ca/renewal` and reuse it in the gRPC handler implementation.
GitOrigin-RevId: 5d8d5aa46279e0ebaf87ed115822359d46ea5a2d
* ca: refactor ChainRenewal handler
Refactor metrics.
GitOrigin-RevId: 3f64145d6fb9c84b54da12fe8245f5a322d3cb4b
* scrypto: drop unmaintained code
Drop unused and unmaintained code from the `go/lib/scrypto` package.
GitOrigin-RevId: f14fc377dd08cebb47be588ebc137dcb676039d1
* launcher: expose SCION build information in metrics
Expose the SCION build information in the prometheus metrics.
A constant gauge with the `version` label is added to the metrics of
every SCION infrastructure element.
```
scion_build_info{version="v0.6.0"} = 1
```
We might add additional information (e.g., commit ID) in the future
as label values.
GitOrigin-RevId: f7c6b39924fcaa7cbb4a7e353329657db20cabf1
* gateway: fix panic when routing policy is not set
GitOrigin-RevId: 04d8550f4a11c0911fa4b72bbd431844d8f54ed0
* ca: Add JWT authorization support
This adds library support for:
- HTTP clients to add a JWT Bearer token to requests
- HTTP servers to perform authorization before running an HTTP handler
The only allowed algorithm is HS256. Servers will make a strict check
and error out if any other algorithm is used.
For security reasons, keys shorted than 256 bits are also not allowed
(see RFC 7518, Section 3.2).
GitOrigin-RevId: 4553063a4dc6a4e4c378e11414ff74c1add79ef7
* gateway: stop monitoring expired paths
Expired paths were not dropped if they could not be updated with fresh paths. This commit fixes that.
GitOrigin-RevId: 3bcd5d7559f9bf09b6e90defeeef43b3b2d8c43d
* TRC: pem encoding of TRC files
All SCION services and tools are now able to handle raw DER and PEM encoded TRCs.
This is achieved by trying to decode PEM of type `TRC`. If that fails, the TRC is treated as raw DER encoded.
GitOrigin-RevId: 79bb7d003a097d073b41330ad906bde8490120a0
* AT: don't send probes when refreshing caches
GitOrigin-RevId: 51ef82da22ec164ec92f8aa979306c78ec11baeb
* AT: support for tshark tracing in CI
GitOrigin-RevId: 1ff08907164e3eedf72b3813bc36337a769771c1
* AT: use standard package installation for tester images
We've used unnecessary complex distroless-style package installation
for tester images. This patch replaces it with the standard Linux
package installation mechanism.
GitOrigin-RevId: 736a3e1c1282226cd52f6eada2203636bb08642c
* renewal: refactor request verifier
Refactor the `RequestVerifier` to make it more reusable.
Furthermore reduce code duplication and improve error messages.
GitOrigin-RevId: cef2b7b57e919648a8c85f1010df9b861a97f67d
* xtest: pem encoding of TRC
Make function LoadTRC accept pem encoded TRC.
GitOrigin-RevId: da69a433dec63048df97ab08a135d2efd6ab2ec6
* bazel-remote: use v1.3.1 version
bazel-remote released a new 2.0.0 version which lead to some issues in
our CI with S3 backed storage.
Therefore we roll back to v1.3.1 for now.
GitOrigin-RevId: cf67088e2f01b00753a26a7e3556e134f044de5f
* ca: add toggles for legacy and cms handlers
Add new toggle to the control service config in the `[ca]` section:
- `disable_legacy_request`: allows disabling the legacy renewal handler.
- `mode`: allows switching between in-process and delegated certificate
renewal mode.
GitOrigin-RevId: 4035572e1bd173c8ee5ae6d4c4f8266de35628b1
* doc: add phases for sensitive TRC update
Add the phases documentation for the sensitive TRC update ceremony.
The sensitive ceremony is executed in `trc_ceremony_sensitive.sh` and is
part of the CI pipeline.
Some additional info is added to the TRC ceremony preparations document
to prevent operators from overwriting existing keys.
GitOrigin-RevId: c1a9f64c714fd5897278942433af3f95b5840784
* fake-gateway: generate fwd path
Support to generate fresh paths provided each hop key.
GitOrigin-RevId: 476490709db72af4e1a0cf0f7b0d57bdfb4bc5e5
* build: use latest master commit of gomock
Previously we used an out of tree commit, which has now been merged.
GitOrigin-RevId: 937717ea61f0c8b293ac841a5e1fd75e135a2e43
* doc: end host bootstrap design proposal
[doc]
Co-authored-by: FR4NK-W <wirzf@inf.ethz.ch>
Closes scionproto#3943
GitOrigin-RevId: b37bfaedd961a870ed1996363c84912ac7095657
* cs: expose registered CA handlers with metrics
renewal_registered_handlers{type="legacy"} 1
renewal_registered_handlers{type="in-process"} 1
renewal_registered_handlers{type="delegating"} 0
Means the legacy and CMS in-process handlers are registered
GitOrigin-RevId: aab74293c369e3f2578f2fc4e5e830dc868507b6
* crypto: add support for storing symmetric keys in PEM format
This PR adds a few utility functions for dealing with symmetric keys
encoded in PEM format.
GitOrigin-RevId: a61d23dd060c59c4c4183d760205c22cd6c32af0
* daemon/snet/showpaths: allow zero for hop latency
Allow to use zero for the latency metadata of each hop. Zero can be a sensible value for this; for example when multiple interfaces are part of the same router instance, then it makes sense to announce zero for the latency between these interfaces.
Use negative value (-1 nanosecond) to represent undefined values internally, instead.
Closes scionproto#4005
GitOrigin-RevId: 51357848a53f5ff7c196c1b426f93084ece3f2cf
* AT: remove curl from tester containers
GitOrigin-RevId: 443586302f3c646ae285453a6d39ed3d8c61cf05
* bazel-remote: use v2.0.1
After having identified and fixed the issue with the 2.0 release,
this is the 2.0 release that should work for us.
GitOrigin-RevId: e1c28de0f15535fa7b0d441fac8dc803b168d33c
* epic-hp: dataplane
This PR implements the EPIC dataplane as described in [1] and [2].
- Define EPIC packet structure.
- Add dataplane handling of EPIC packets.
- Add unit tests for packet parsing, EPIC functionality, and the dataplane.
Changes to existing code:
- mac.go: add function FullMAC(), which returns the full 16 bytes (instead of only 6 bytes) of the MAC.
- dataplane.go: add processEPIC() function.
- dataplane.go: add "cachedMac" field to the scionPacketProcessor, which allows to cache the (full 16 bytes of the) hop field MAC, such that it does not need to be recalculated by EPIC.
Todo for later:
- [x] Replace CMAC with CBC-MAC in PHVF/LHVF verification.
- [ ] Send back SCMP messages when PHVF/LHVF verification fails (at the moment the packet is just dropped).
[1] https://scion.docs.anapaya.net/en/latest/EPIC.html#data-plane
[2] https://scion.docs.anapaya.net/en/latest/protocols/scion-header.html#path-type-epic-hp
Closes scionproto#3951
GitOrigin-RevId: 7a2cb0a46217b888fc2ee72f48fe9f656d7de5ef
* ca: add support for certificate renewal delegation
Also add support for keeping in-memory objects synchronized with
disk file contents.
GitOrigin-RevId: ae3c9a3bc8d0e50bf92694e5ef753d3f645d39a8
* epic-dp: fix unit test flakyness
The cachedEpicTS variable was accessed by multiple tests in parallel
which lead to flakyness. Instead of the caching approach fix the time in
the beginning of the test and inject it.
GitOrigin-RevId: e185b32b4378691bf4411b58acc0de1e855172c2
Co-authored-by: Dominik Roos <roos@anapaya.net>
Co-authored-by: Martin Sustrik <sustrik@anapaya.net>
Co-authored-by: Samuel Hitz <hitz@anapaya.net>
Co-authored-by: noorizea <60094353+noorizea@users.noreply.github.com>
Co-authored-by: gbrel <cristache@anapaya.net>
Co-authored-by: mawyss <mawyss@ethz.ch>
Co-authored-by: Konstantinos <karampogias@anapaya.net>
Co-authored-by: Sergiu Costea <sergiu.costea@gmail.com>
Co-authored-by: Lukas Vogel <vogel@anapaya.net>
Co-authored-by: Sergio Gonzalez Monroy <monroy@anapaya.net>
Co-authored-by: Andrea Tulimiero <tulimiero.andrea@gmail.com>
Co-authored-by: FR4NK-W <wirzf@inf.ethz.ch>
Co-authored-by: Matthias Frei <matthias.frei@inf.ethz.ch>
This was referenced Nov 22, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Allow to use zero for the latency metadata of each hop. Zero can be a sensible value for this; for example when multiple interfaces are part of the same router instance, then it makes sense to announce zero for the latency between these interfaces.
Use negative value (-1 nanosecond) to represent undefined values internally, instead.
This change is