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

Llt 5309 integrate multicast components #661

Merged
merged 2 commits into from
Aug 9, 2024

Conversation

ThrasherLT
Copy link
Contributor

@ThrasherLT ThrasherLT commented Jul 5, 2024

Problem

All the separate parts of starcast (see RFC LLT-0060) are ready to be integrated into Telio.

Solution

This PR integrates all the separate starcast parts (virtual peer, transport and nat) into libtelio.
You can see the doc rs in code of every separate component to understand how they're supposed to be integrated into telio.

The Nat-Lab test for multicast is being implemented with a separate PR, so this one is untested yet.

☑️ Definition of Done checklist

  • Commit history is clean (requirements)
  • README.md is updated
  • Functionality is covered by unit or integration tests

@ThrasherLT ThrasherLT force-pushed the LLT-5309_Integrate_multicast_components branch 3 times, most recently from 0f9ce6e to 1b8d6f8 Compare July 9, 2024 13:22
@ThrasherLT ThrasherLT marked this pull request as ready for review July 9, 2024 13:30
@ThrasherLT ThrasherLT requested a review from a team as a code owner July 9, 2024 13:30
@LukasPukenis
Copy link
Contributor

As a reviewer I can't effectively review because while the PR description tells what this PR does(I understand words), it doesn't add any context, thus starcast for me is just a buzzword :/ It would really help to have more information what it is

@ThrasherLT ThrasherLT force-pushed the LLT-5309_Integrate_multicast_components branch 2 times, most recently from e5a69ab to b8373de Compare July 11, 2024 10:45
@mathiaspeters
Copy link
Contributor

thus starcast for me is just a buzzword :/

In a nutshell, starcast (*cast was the working name to cover multicast and broadcast as one word, and it could be pronounced as starcast if you say it as an actual word) is the meshnet implementation of multicast. I guess this is a general problem with "integration PRs" where you need to understand the components that are being integrated to properly understand and review the integration work. For more background you can check RFCs LLT-0060 and LLT-0047, or if it makes more sense, this could be a candidate for a review meeting or something like that to make it easier for you and to share some knowledge

@jjanowsk jjanowsk force-pushed the LLT-5309_Integrate_multicast_components branch 3 times, most recently from e3747ab to cc44505 Compare July 17, 2024 11:10
Copy link
Collaborator

@packgron packgron left a comment

Choose a reason for hiding this comment

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

Few nit's

@mathiaspeters
Copy link
Contributor

+1

@jjanowsk jjanowsk force-pushed the LLT-5309_Integrate_multicast_components branch 4 times, most recently from d2a78aa to 257f9f5 Compare July 26, 2024 07:50
Copy link
Collaborator

@packgron packgron left a comment

Choose a reason for hiding this comment

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

+1.0

@jjanowsk jjanowsk force-pushed the LLT-5309_Integrate_multicast_components branch from 257f9f5 to 5a3b037 Compare August 2, 2024 10:59
packgron
packgron previously approved these changes Aug 2, 2024
@jjanowsk jjanowsk enabled auto-merge August 2, 2024 11:40
@jjanowsk jjanowsk force-pushed the LLT-5309_Integrate_multicast_components branch 2 times, most recently from cd4a428 to 411bafd Compare August 9, 2024 06:05
@jjanowsk jjanowsk force-pushed the LLT-5309_Integrate_multicast_components branch from 411bafd to a5c91ff Compare August 9, 2024 08:01
@jjanowsk jjanowsk merged commit be70d57 into main Aug 9, 2024
58 of 59 checks passed
@jjanowsk jjanowsk deleted the LLT-5309_Integrate_multicast_components branch August 9, 2024 09:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants