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

ipv6 support #69

Closed
lu-zero opened this issue Jan 18, 2021 · 3 comments · Fixed by #100
Closed

ipv6 support #69

lu-zero opened this issue Jan 18, 2021 · 3 comments · Fixed by #100
Labels
good first issue Good for newcomers

Comments

@lu-zero
Copy link
Contributor

lu-zero commented Jan 18, 2021

I saw that the current master has at least a TODO about it in srt-protocol/src/packet/control.rs:

                // TODO: this is probably really wrong, so fix it
                let peer_addr = if ip_buf[4..] == [0; 12][..] {
                    IpAddr::from(Ipv4Addr::new(ip_buf[3], ip_buf[2], ip_buf[1], ip_buf[0]))
                } else {
                    IpAddr::from(ip_buf)
                };

Is there anything else to be addressed?

@russelltg
Copy link
Owner

Yeah, so I'm pretty sure the "correct" way to do this the packet needs to know if it's being send/received from an ipv6 socket, so it will require a bit of reorganization (right now it just "guesses" by the lower bytes in the ipv6 address)

@russelltg
Copy link
Owner

russelltg commented Jan 18, 2021

For anyone who would want to take this issue:

  • Look at the reference implementation's treatment of this, make sure my assumption is correct
  • Allow packet parsing to know if it's IPV4 or IPV6 (or, alternatively, just store the IP buffer in the packet and rely on the implementation to know), I'm willing to be convinced either way. I'd prefer the first if it's not too messy, to keep the packet structure "pure"
  • Add tests using IPv6

@russelltg russelltg added the good first issue Good for newcomers label Jan 18, 2021
@lu-zero
Copy link
Contributor Author

lu-zero commented Jan 19, 2021

I can confirm that the it seem so:

// in srtcore/core.cpp
    CIPAddress::pton((m_parent->m_SelfAddr), m_piSelfIP, agent.family(), peer);

lu-zero added a commit to lu-zero/srt-rs that referenced this issue Jan 20, 2021
@russelltg russelltg mentioned this issue May 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants