BlueOS icon indicating copy to clipboard operation
BlueOS copied to clipboard
verified publisher icon bluerobotics

Revert removal of IPv6

Open rotu opened this issue 10 months ago • 3 comments

Revert commits:

  • 62fbcb4279cd32e3c4bad3e25d5596986ab37aa8 wifi: networkmanager: disable ipv6
  • 715af4e8b0d3805a9b4b685417641410f0587a6a core:tools:blueos_startup_update: No IPV6

It is incorrect to disable IPv6 and unclear why this was done in the first place. I believe the underlying motivation of this change may have been DHCP4 issues which these PRs did not properly address. #1991

Summary by Sourcery

Reverts the disabling of IPv6 in the blueos_startup_update tool and the wifi service.

rotu avatar Mar 04 '25 19:03 rotu

Reviewer's Guide by Sourcery

This pull request reverts the changes introduced by commit 715af4e8b0d3805a9b4b685417641410f0587a6a, effectively re-enabling IPv6. The changes involve removing the IPv6 disabling logic from the blueos_startup_update.py script and the wifi connection setup in networkmanager.py.

No diagrams generated as the changes look simple and do not need a visual representation.

File-Level Changes

Change Details Files
Reverts the disabling of IPv6 in the system configuration.
  • Removes the function ensure_ipv6_disabled from blueos_startup_update.py. This function was responsible for ensuring IPv6 was disabled by modifying /etc/sysctl.conf.
  • Removes the call to ensure_ipv6_disabled from the main function in blueos_startup_update.py.
  • Removes the IPv6 configuration from the wifi connection setup in networkmanager.py.
 core/tools/blueos_startup_update/blueos_startup_update.py
core/services/wifi/wifi_handlers/networkmanager/networkmanager.py
Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an issue from a review comment by replying to it. You can also reply to a review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull request title to generate a title at any time. You can also comment @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in the pull request body to generate a PR summary at any time exactly where you want it. You can also comment @sourcery-ai summary on the pull request to (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the pull request to resolve all Sourcery comments. Useful if you've already addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull request to dismiss all existing Sourcery reviews. Especially useful if you want to start fresh with a new review - don't forget to comment @sourcery-ai review to trigger a new review!
  • Generate a plan of action for an issue: Comment @sourcery-ai plan on an issue to generate a plan of action for it.

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

  • Contact our support team for questions or feedback.
  • Visit our documentation for detailed guides and information.
  • Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

sourcery-ai[bot] avatar Mar 04 '25 19:03 sourcery-ai[bot]

I don''t disagree that this is "wrong" in principle. The issue is that historically, ipv6 has given us more issues that it has helped, and we don't current have enough workforce to address the issue fully.

This patch will likely case more issues as it will result in a mix of ipv4+ipv6 and ipv4-only systems. the correct fix would be to actually have a patch reverting the systems which are already ipv4-only, too.

Meanwhile, I recommend relying on https://github.com/bluerobotics/BlueOS/pull/3049 to disable the "noIPV6" patch

Williangalvani avatar Mar 06 '25 16:03 Williangalvani

The issue is that historically, ipv6 has given us more issues that it has helped, and we don't current have enough workforce to address the issue fully.

Even so, I'm not sure that this code correctly addresses issues caused by IPv6.

Loopback is still using IPv6 so I think that line net.ipv6.conf.lo.disable_ipv6=1 is not working at all by default. localhost will still have any problems that you associate with IPv6.

Disabling IPv6 on Ethernet causes Windows to prefer literally any other interface that has IPv6, even if it has a higher metric (like wifi)! I'm now realizing that trying to disable IPv4 is responsible for blueos-avahi.local preferring the wifi interface and zerotier over the ethernet adapter. AFAICT, the reason you need to edit the wifi connection is to fix the problems caused by disabling IPv6 on ethernet.

I think the right fix, if you want to not accidentally use IPv6 is to address the host using a static IPv4 address. If you want to use automatic routing with multiple network interfaces, then the solution is to to configure the MDNS daemon appropriately.

the correct fix would be to actually have a patch reverting the systems which are already ipv4-only, too.

Which systems are ipv4-only?

rotu avatar Mar 06 '25 20:03 rotu