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

Attempt to read ipv4-mapped ipv6 to ipv4 if possible #3221

Merged
merged 1 commit into from
Feb 14, 2020

Conversation

quentinlesceller
Copy link
Member

A potential additional operation during read some maybe not the best idea.

Copy link
Member

@antiochp antiochp left a comment

Choose a reason for hiding this comment

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

Majority of our peer addresses are currently ipv4 and this additional step is only used for ipv6 - so I think that is probably fine.
I think this makes sense - but need to think it through a bit more.

Copy link
Member

@antiochp antiochp left a comment

Choose a reason for hiding this comment

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

We need to work out where these ipv6 addresses are coming from as well.

  1. outbound peer connection (remote peer accepts both ipv4/ipv6 so we see a mapped version)
  2. inbound peer connection (peer connects to us, peer configured for both ipv4/ipv6)
  3. via a broadcast list of peer addresses?

I'm not even sure we would use this code path if it was (1) or (2) as we read the peer address via the underlying tcp connection? At least for one of those, maybe not the other.

@quentinlesceller
Copy link
Member Author

Only case that I have seen is 3. This PR do the job in that case.

@antiochp
Copy link
Member

antiochp commented Feb 6, 2020

So I think we also need similar logic when we compare ip addrs when looking to see if we are already connected to a peer.
We do this when accepting a connection (and maybe when trying to connect). If we are already connected then we want to identify the duplicate attempt and close the new connection. And I believe we do this based on a comparison of the ip address.

@antiochp
Copy link
Member

antiochp commented Feb 6, 2020

Related - #3208.

@antiochp
Copy link
Member

antiochp commented Feb 6, 2020

If we can get to point where we are confident this improves the overall situation regarding ipv4-mapped addresses on the network then we should aim to get this merged in for a potential 3.1.0 release.

Copy link
Member

@antiochp antiochp left a comment

Choose a reason for hiding this comment

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

I think this in combination with #3225 is a good idea.

👍

@quentinlesceller quentinlesceller merged commit b400a4b into mimblewimble:master Feb 14, 2020
@quentinlesceller quentinlesceller deleted the ipv6 branch February 14, 2020 14:37
@antiochp antiochp mentioned this pull request Feb 24, 2020
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