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

docs: vpn: Add documentation for evaluation of vpn protocols #3267

Merged
merged 1 commit into from
Jul 10, 2024

Conversation

maurerle
Copy link
Member

@maurerle maurerle commented May 29, 2024

T-X created a pretty cool table to evaluate VPN uplink protocols used in gluon on this hedgedoc:
https://md.chaotikum.org/zRkW6JXXQs-8WCnwdtig5w?view

To give it more visibility, I would like to add it to the docs as suggested somewhere.

I hope this is fine for you @T-X and @rotanid ?

@github-actions github-actions bot added the 3. topic: docs Topic: Documentation label May 29, 2024
@maurerle maurerle changed the title Add documentation for evaluation of mesh protocols docs: vpn: Add documentation for evaluation of mesh protocols May 29, 2024
@rotanid
Copy link
Member

rotanid commented May 30, 2024

my first thought was, that your text and commit message talk about "mesh protocol" while the content isn't about mesh protocols but about VPNs ;-)

@T-X
Copy link
Contributor

T-X commented May 31, 2024

@maurerle oh, had totally forgotten about this table already. I like the idea of adding it to the docs, thanks for the conversion effort!

I think "fastd, null@l2tp, offloaded" should have "Single interface for all peers" -> "no" though? I'm also wondering if it could be nice to add a link to the Mesh VPN MTU page onto the "MTU overhead" heading?

Also would be nice if someone could do a quick check for the l2tp kernel module if it supports multi-threading, on some live device or x86 VM.

@T-X
Copy link
Contributor

T-X commented May 31, 2024

I'm also wondering what to do with the Tunneldigger entry. As all of its mentions was removed from the docs otherwise. Maybe add a link to the according community package on Github onto "Tunneldigger" and add "inofficial" in brackets? (or even "deprecated"?)

Edit: As no new maintainer of it has stepped up so far, maybe "deprecated" might be more appropriate for now?

@T-X
Copy link
Contributor

T-X commented May 31, 2024

And another thought, would it maybe make sense to place it higher instead of at the bottom as it's a nice, quick, first overview?

(Edit: and then rename it to "Protocols Overview"?)

@T-X
Copy link
Contributor

T-X commented May 31, 2024

"former supported" -> "formerly supported" (adverb)

and add "ones" after "formerly supported"?

Also no comma in this first sentence. (rule in English: if the sentence/clause can't exist on its own then no comma; I know, unintuitive for us native German speakers, where we use a lot more commas :D)

@maurerle maurerle changed the title docs: vpn: Add documentation for evaluation of mesh protocols docs: vpn: Add documentation for evaluation of vpn protocols May 31, 2024
Copy link
Contributor

@grische grische left a comment

Choose a reason for hiding this comment

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

Some suggestions

docs/features/vpn.rst Outdated Show resolved Hide resolved
docs/features/vpn.rst Outdated Show resolved Hide resolved
docs/features/vpn.rst Outdated Show resolved Hide resolved
docs/features/vpn.rst Outdated Show resolved Hide resolved
docs/features/vpn.rst Outdated Show resolved Hide resolved
docs/features/vpn.rst Outdated Show resolved Hide resolved
docs/features/vpn.rst Outdated Show resolved Hide resolved
@maurerle
Copy link
Member Author

maurerle commented Jul 9, 2024

Thanks @rotanid , @T-X and @grische - I updated the PR according to your review and suggestion.
I hope we can merge this soon :)

@grische
Copy link
Contributor

grische commented Jul 10, 2024

@maurerle I can't see any changes. Did you forget to push the commits?

@maurerle
Copy link
Member Author

Sorry, I pushed to the wrong repository (ffac).. Thanks

Copy link
Contributor

@grische grische left a comment

Choose a reason for hiding this comment

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

There's a comma missing, otherwise LGTM.

docs/features/vpn.rst Outdated Show resolved Hide resolved
docs/features/vpn.rst Outdated Show resolved Hide resolved
T-X created a pretty cool table to evaluate mesh-vpn protocols on this hedgedoc:
https://md.chaotikum.org/zRkW6JXXQs-8WCnwdtig5w?view

To give it more visibility, I would like to add it to the docs as suggested somewhere
@blocktrron blocktrron merged commit dc51f3d into freifunk-gluon:main Jul 10, 2024
10 checks passed
@maurerle maurerle deleted the vpn_protocol_table branch July 10, 2024 14:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. topic: docs Topic: Documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants