Skip to content

refactor: use netip.AddrPort #3816

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

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

alexandear
Copy link
Member

This PR changes the fields of iptables.Entry from IP net.IP and Port int to AddrPort netip.AddrPort. This results in slightly simpler code, and iptables.Entry becomes comparable.

net.IP is not comparable because slices in Go are not comparable.

@alexandear alexandear force-pushed the refactor/iptables-netipaddrport branch 2 times, most recently from 3ff26f2 to 63711de Compare August 11, 2025 13:51
Signed-off-by: Oleksandr Redko <[email protected]>
@alexandear alexandear force-pushed the refactor/iptables-netipaddrport branch from 63711de to e904997 Compare August 11, 2025 17:27
@jandubois
Copy link
Member

I can't spot where the error comes from, but port-forwarding tests are failing.

With Fedora:

❌ Forwarding TCP from [::]:4024 to 10.1.0.209:4024
     Guest received: ''

It is not always the same one that fails though:

 ❌ Forwarding TCP from 127.0.0.2:4031 to 10.1.0.209:4031
     Guest received: ''

With Alpine:

 ❌ Forwarding TCP from 127.0.0.2:4021 to 10.1.0.142:4021
     Guest received: ''

I will run the tests one more time, but I think the same set of 3 tests failed both times.

@alexandear
Copy link
Member Author

I can't spot where the error comes from, but port-forwarding tests are failing.

With Fedora:

❌ Forwarding TCP from [::]:4024 to 10.1.0.209:4024
     Guest received: ''

It is not always the same one that fails though:

 ❌ Forwarding TCP from 127.0.0.2:4031 to 10.1.0.209:4031
     Guest received: ''

With Alpine:

 ❌ Forwarding TCP from 127.0.0.2:4021 to 10.1.0.142:4021
     Guest received: ''

I will run the tests one more time, but I think the same set of 3 tests failed both times.

I'll take a look, thanks.

@alexandear alexandear marked this pull request as draft August 12, 2025 15:27
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.

2 participants