Open
Conversation
taktv6
reviewed
Dec 12, 2024
Ipv6 support
fix bug with the configuration parsing and tests
early, avoid unsuccessful port bindings
Add omitempty to yaml, speed up launching probers, start http server early, avoid unsuccessful port bindings
Detect undefined hops in paths
allow empty ip in listen_address
remove unused import
Add support for per path custom labels
Config: Reload config automatically
Config: do not expose internal fields
ns annotation
Improve efficiency of prober.Marshal
labels fix typo
fix autoreload
Author
|
Current limitation: The source address for the IPv6 packages can't be spoofed. |
mraerino
reviewed
Jun 26, 2025
Comment on lines
+21
to
+37
| ` | ||
| term DECAP { | ||
| from { | ||
| destination-address { | ||
| 10.1.0.255/32; | ||
| } | ||
| protocol gre; | ||
| } | ||
| then { | ||
| decapsulate gre; | ||
| } | ||
| } | ||
| term ACCEPT { | ||
| then accept; | ||
| } | ||
| } | ||
| ` |
There was a problem hiding this comment.
this should probably be a code block with three backticks to improve readability
Comment on lines
+39
to
+43
| `# ip tunnel add gre_decap mode gre local 192.0.2.0 ttl 255 | ||
| ``# echo 1 > /proc/sys/net/ipv4/conf/ip_forwarding | ||
| `Necessary if you can’t spoof addresses: | ||
| ``# echo 1 > /proc/sys/net/ipv4/conf/<input_dev>/accept_local | ||
| ` |
There was a problem hiding this comment.
i'd also recommand using a code block with three backticks here
|
|
||
| err = pm.Configure(cfg) | ||
| if err != nil { | ||
| log.Errorf("reconfiguration failed: %v", err) |
There was a problem hiding this comment.
should this also be fatal or is the service able to start up without a configured prober manager?
Comment on lines
+127
to
+144
| ipVersion := p.cfg.IPVersion | ||
|
|
||
| if ipVersion == 4 { | ||
| rc, err := newRawSockWrapper() | ||
| if err != nil { | ||
| return fmt.Errorf("Unable to create rack socket wrapper: %v", err) | ||
| } | ||
|
|
||
| p.rawConn = rc | ||
| } | ||
|
|
||
| if ipVersion == 6 { | ||
| rc, err := newIPv6RawSockWrapper() | ||
| if err != nil { | ||
| return fmt.Errorf("Unable to create rack socket wrapper: %v", err) | ||
| } | ||
|
|
||
| p.rawConn = rc |
There was a problem hiding this comment.
nit: this could be more concise
Suggested change
| ipVersion := p.cfg.IPVersion | |
| if ipVersion == 4 { | |
| rc, err := newRawSockWrapper() | |
| if err != nil { | |
| return fmt.Errorf("Unable to create rack socket wrapper: %v", err) | |
| } | |
| p.rawConn = rc | |
| } | |
| if ipVersion == 6 { | |
| rc, err := newIPv6RawSockWrapper() | |
| if err != nil { | |
| return fmt.Errorf("Unable to create rack socket wrapper: %v", err) | |
| } | |
| p.rawConn = rc | |
| var rc rawSocket | |
| var err error | |
| switch p.cfg.IPVersion { | |
| case 4: | |
| rc, err = newRawSockWrapper() | |
| case 6: | |
| rc, err = newIPv6RawSockWrapper() | |
| default: | |
| return fmt.Errorf("invalid IP version: %d", p.cfg.IPVersion) | |
| } | |
| if err != nil { | |
| return fmt.Errorf("Unable to create raw socket wrapper: %v", err) | |
| } | |
| p.rawConn = rc |
Comment on lines
+31
to
+42
| func (pm *ProberManager) Configure(cfg *config.Config) error { | ||
| pm.probersMu.Lock() | ||
| defer pm.probersMu.Unlock() | ||
|
|
||
| desiredProbers, err := getDesiredProbers(cfg) | ||
| if err != nil { | ||
| return fmt.Errorf("unable to create probers: %v", err) | ||
| } | ||
|
|
||
| pm.adjustProbers(desiredProbers) | ||
| return nil | ||
| } |
There was a problem hiding this comment.
if you want to reduce the time the mutex is exclusively locked, consider moving it around the part that actually needs the locking. afaict getDesiredProbers doesn't need any locking
alternatively you can also move the locking into adjustProbers
Suggested change
| func (pm *ProberManager) Configure(cfg *config.Config) error { | |
| pm.probersMu.Lock() | |
| defer pm.probersMu.Unlock() | |
| desiredProbers, err := getDesiredProbers(cfg) | |
| if err != nil { | |
| return fmt.Errorf("unable to create probers: %v", err) | |
| } | |
| pm.adjustProbers(desiredProbers) | |
| return nil | |
| } | |
| func (pm *ProberManager) Configure(cfg *config.Config) error { | |
| desiredProbers, err := getDesiredProbers(cfg) | |
| if err != nil { | |
| return fmt.Errorf("unable to create probers: %v", err) | |
| } | |
| pm.probersMu.Lock() | |
| pm.adjustProbers(desiredProbers) | |
| pm.probersMu.Unlock() | |
| return nil | |
| } |
Recreate prober if labels have changed
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
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.
Add general documentation about the project and some auto-generated one for the configuration file