Skip to content

Filter Discovered peers for ipv6 support#6498

Merged
garyschulte merged 10 commits intobesu-eth:mainfrom
garyschulte:bugfix/discovery-ipv6-errors
Jan 31, 2024
Merged

Filter Discovered peers for ipv6 support#6498
garyschulte merged 10 commits intobesu-eth:mainfrom
garyschulte:bugfix/discovery-ipv6-errors

Conversation

@garyschulte
Copy link
Contributor

PR description

updates PeerDiscoveryAgent to

  • use existing NetworkUtility to filter for any/none peers
  • fall back to source address if ipv6 is not supported (like in a docker container)

Fixed Issue(s)

fixes #6475

@github-actions
Copy link

  • I thought about documentation and added the doc-change-required label to this PR if updates are required.
  • I thought about the changelog and included a changelog update if required.
  • If my PR includes database changes (e.g. KeyValueSegmentIdentifier) I have thought about compatibility and performed forwards and backwards compatibility tests

@garyschulte garyschulte force-pushed the bugfix/discovery-ipv6-errors branch from b9d08c7 to ebf915b Compare January 31, 2024 01:48
@macfarla macfarla requested a review from pinges January 31, 2024 02:18
@garyschulte garyschulte force-pushed the bugfix/discovery-ipv6-errors branch from d9d5aab to 260d6b4 Compare January 31, 2024 02:51
Copy link
Contributor

@pinges pinges left a comment

Choose a reason for hiding this comment

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

Left two comments, but LGTM

.orElseGet(sourceEndpoint::getHost);
.orElseGet(
() -> {
LOG.trace(
Copy link
Contributor

Choose a reason for hiding this comment

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

LOG.atTrace would be nice ...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

since this is already in a lambda, the extra log lambda isn't necessary :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh wait, yeah. you are right! I originally had this logging every time, but it was noisy and moved it to trace

@@ -173,4 +196,20 @@ private static boolean isPortAvailableForUdp(final int port) {
public static boolean isPortAvailable(final int port) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I've had a look at the isPortAvailableFor[Tcp|Udp] and I thought that we might want to close the ServerSocket if we successfully binded to it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Apparently SO_REUSEADDR makes it moot, but I don't see why we wouldn't unbind. It isn't in the scope of this PR, but I can put up a subsequent one.

…ltering, add ipv6 check/fallback

Signed-off-by: garyschulte <garyschulte@gmail.com>
Signed-off-by: garyschulte <garyschulte@gmail.com>
Signed-off-by: garyschulte <garyschulte@gmail.com>
Signed-off-by: garyschulte <garyschulte@gmail.com>
Signed-off-by: garyschulte <garyschulte@gmail.com>
Signed-off-by: garyschulte <garyschulte@gmail.com>
Signed-off-by: garyschulte <garyschulte@gmail.com>
@garyschulte garyschulte force-pushed the bugfix/discovery-ipv6-errors branch from 260d6b4 to 7dc8bc7 Compare January 31, 2024 03:51
@garyschulte garyschulte enabled auto-merge (squash) January 31, 2024 03:51
Signed-off-by: garyschulte <garyschulte@gmail.com>
@garyschulte garyschulte force-pushed the bugfix/discovery-ipv6-errors branch from 1757144 to 59da07a Compare January 31, 2024 03:58
Signed-off-by: garyschulte <garyschulte@gmail.com>
Signed-off-by: garyschulte <garyschulte@gmail.com>
@garyschulte garyschulte disabled auto-merge January 31, 2024 04:07
@garyschulte garyschulte enabled auto-merge (squash) January 31, 2024 04:07
@garyschulte garyschulte merged commit 79245bb into besu-eth:main Jan 31, 2024
Gabriel-Trintinalia pushed a commit to Gabriel-Trintinalia/besu that referenced this pull request Feb 1, 2024
* use existing NetworkUtility for PeerDiscoveryAgent pingpacket data filtering, add ipv6 check/fallback
* log at debug when we override pingpacket from
* use java native address parsing rather than lookup by host

Signed-off-by: garyschulte <garyschulte@gmail.com>
Signed-off-by: Gabriel-Trintinalia <gabriel.trintinalia@consensys.net>
@garyschulte garyschulte deleted the bugfix/discovery-ipv6-errors branch April 3, 2024 19:25
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.

IPv6 peer discovery errors

2 participants