Skip to content

Conversation

@mratmeyer
Copy link

This PR adds a few changes to make LZR compatible with IPv6:

  • New ipv6Enabled flag, false by default which enables support for IPv6
  • Updates BPF filter in IPv6 mode
  • Adds brackets around host header to be compatible with IPv6
  • Explode IPv6 IPs

These changes were made by @merterdemir, @GQW19, and myself.

Copy link
Contributor

@merterdemir merterdemir left a comment

Choose a reason for hiding this comment

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

General review to see if anything has been missed or not. Might need to fix the input delimiter to avoid crashes in the IPv6.

packet.go Outdated
//expecting ip, port
input = strings.TrimSuffix(input, "\n")
s := strings.Split(input,":")
s := strings.Split(input, ":")
Copy link
Contributor

Choose a reason for hiding this comment

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

This might fail with the IPv6 hosts as they use : character within the address string. I would suggest changing this to:

s := strings.Split(input, ";")

and using ; as the new delimiter (or ,).

Copy link
Author

@mratmeyer mratmeyer Sep 3, 2025

Choose a reason for hiding this comment

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

Oh that’s right, good catch. So would it be better to change only the IPv6 delimeter to ; or should we consider letting the ; delimeter work on IPv4 as well? It could be confusing for users to need ; for IPv6 but : for IPv4. Maybe should be a question for the maintainers along with which delimeter character they’d prefer

Copy link
Contributor

Choose a reason for hiding this comment

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

I added this change to make sure it works for IPv6 and couple more missing changes on the packet construction routine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants