Skip to content

warn on uselessly low MTU - #1797

Draft
JackDoan wants to merge 1 commit into
masterfrom
mtu-whiner
Draft

warn on uselessly low MTU#1797
JackDoan wants to merge 1 commit into
masterfrom
mtu-whiner

Conversation

@JackDoan

Copy link
Copy Markdown
Collaborator

No description provided.

Comment thread examples/config.yml
Comment thread hostmap.go
}

func localAddrs(l *slog.Logger, allowList *LocalAllowList) []localAddr {
//FIXME: This function is pretty garbage

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

<3

Comment thread lighthouse.go

const (
// Both AES-256-GCM and ChaCha20-Poly1305 append a 16 byte AEAD tag
cipherTagLen = 16

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We should get this from the current cipher, will be slightly annoying to thread it through though. Keeps us from the future footgun.

Comment thread lighthouse.go

// checkLocalLinkMTU classifies e's link MTU, logs when the classification
// changes, and reports whether e should be advertised to lighthouses.
func (lh *LightHouse) checkLocalLinkMTU(e localAddr) bool {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Feels like all of this can move to its own package and relieve lighthouse.go of carrying it along.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

where do you wanna draw the line? I agree but its super blurry

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The stateless stuff. Feels like header should have the bits that matter for the header + relay, cipher config has the tag len (using a const there is future pain if we can avoid it now), the bits with state should stay in lighthouse.go, the rest can live somewhere else.

Like the stuff in hostmap.go, no reason for it, just never moved it anywhere else.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants