Skip to content

Conversation

OkamiW
Copy link
Contributor

@OkamiW OkamiW commented Jun 25, 2025

For TCP forwarder, we can reset the connection with r.Complete(true). For UDP forwarder, there was no way to reset the connection.

gVisor already sends ICMP port unreachable for unhandled packets, but udp.Forwarder.HandlePacket always returns true.

So we change the handler's function signature to return a bool type value, indicating whether the packet was handled or not. Thus allowing us to send ICMP port unreachable for failed UDP connections.

Copy link

google-cla bot commented Jun 25, 2025

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

Copy link
Collaborator

@ayushr2 ayushr2 left a comment

Choose a reason for hiding this comment

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

udp.NewForwarder() has no non-test usages in gVisor...

Is this dead code we need to clean up? Or are you a Netstack package user and still want this functionality?

@OkamiW OkamiW force-pushed the udp-forwarder-not-handled-packets branch from 59c8c5a to 9912f38 Compare June 26, 2025 05:36
@OkamiW
Copy link
Contributor Author

OkamiW commented Jun 26, 2025

udp.NewForwarder() has no non-test usages in gVisor...

Is this dead code we need to clean up? Or are you a Netstack package user and still want this functionality?

By Netstack, do you mean https://github.com/google/netstack?
I'm not using github.com/google/netstack, I'm using github.com/google/gvisor directly in my package:
https://github.com/OkamiW/proxy-ns/blob/master/go.mod#L9

This functionality is very useful for handling packets coming from a TUN interface.

@ayushr2
Copy link
Collaborator

ayushr2 commented Jun 26, 2025

By netstack, I meant https://github.com/google/gvisor/tree/master/pkg/tcpip. The netstack repo you linked was internalized into the gVisor repo some time back.

@avagin
Copy link
Collaborator

avagin commented Jul 1, 2025

pls fix the cla/google check

@OkamiW
Copy link
Contributor Author

OkamiW commented Jul 1, 2025

pls fix the cla/google check

Done.

copybara-service bot pushed a commit that referenced this pull request Jul 1, 2025
For TCP forwarder, we can reset the connection with r.Complete(true). For UDP forwarder, there was no way to reset the connection.

gVisor already sends ICMP port unreachable for unhandled packets, but udp.Forwarder.HandlePacket always returns true.

So we change the handler's function signature to return a bool type value, indicating whether the packet was handled or not. Thus allowing us to send ICMP port unreachable for failed UDP connections.

FUTURE_COPYBARA_INTEGRATE_REVIEW=#11850 from OkamiW:udp-forwarder-not-handled-packets 9912f38
PiperOrigin-RevId: 777960633
@OkamiW
Copy link
Contributor Author

OkamiW commented Jul 7, 2025

Gentle ping.

For TCP forwarder, we can reset the connection with r.Complete(true).
For UDP forwarder, there was no way to reset the connection.

gVisor already sends ICMP port unreachable for unhandled packets, but
udp.Forwarder.HandlePacket always returns true.

So we change the handler's function signature to return a bool type
value, indicating whether the packet was handled or not. Thus allowing
us to send ICMP port unreachable for failed UDP connections.
@OkamiW OkamiW force-pushed the udp-forwarder-not-handled-packets branch from 9912f38 to 496c218 Compare July 9, 2025 11:33
copybara-service bot pushed a commit that referenced this pull request Jul 9, 2025
For TCP forwarder, we can reset the connection with r.Complete(true). For UDP forwarder, there was no way to reset the connection.

gVisor already sends ICMP port unreachable for unhandled packets, but udp.Forwarder.HandlePacket always returns true.

So we change the handler's function signature to return a bool type value, indicating whether the packet was handled or not. Thus allowing us to send ICMP port unreachable for failed UDP connections.

FUTURE_COPYBARA_INTEGRATE_REVIEW=#11850 from OkamiW:udp-forwarder-not-handled-packets 496c218
PiperOrigin-RevId: 777960633
copybara-service bot pushed a commit that referenced this pull request Jul 9, 2025
For TCP forwarder, we can reset the connection with r.Complete(true). For UDP forwarder, there was no way to reset the connection.

gVisor already sends ICMP port unreachable for unhandled packets, but udp.Forwarder.HandlePacket always returns true.

So we change the handler's function signature to return a bool type value, indicating whether the packet was handled or not. Thus allowing us to send ICMP port unreachable for failed UDP connections.

FUTURE_COPYBARA_INTEGRATE_REVIEW=#11850 from OkamiW:udp-forwarder-not-handled-packets 496c218
PiperOrigin-RevId: 777960633
copybara-service bot pushed a commit that referenced this pull request Jul 9, 2025
For TCP forwarder, we can reset the connection with r.Complete(true). For UDP forwarder, there was no way to reset the connection.

gVisor already sends ICMP port unreachable for unhandled packets, but udp.Forwarder.HandlePacket always returns true.

So we change the handler's function signature to return a bool type value, indicating whether the packet was handled or not. Thus allowing us to send ICMP port unreachable for failed UDP connections.

FUTURE_COPYBARA_INTEGRATE_REVIEW=#11850 from OkamiW:udp-forwarder-not-handled-packets 496c218
PiperOrigin-RevId: 777960633
@OkamiW
Copy link
Contributor Author

OkamiW commented Jul 14, 2025

Gentle ping.

copybara-service bot pushed a commit that referenced this pull request Jul 14, 2025
For TCP forwarder, we can reset the connection with r.Complete(true). For UDP forwarder, there was no way to reset the connection.

gVisor already sends ICMP port unreachable for unhandled packets, but udp.Forwarder.HandlePacket always returns true.

So we change the handler's function signature to return a bool type value, indicating whether the packet was handled or not. Thus allowing us to send ICMP port unreachable for failed UDP connections.

FUTURE_COPYBARA_INTEGRATE_REVIEW=#11850 from OkamiW:udp-forwarder-not-handled-packets 496c218
PiperOrigin-RevId: 783025458
@copybara-service copybara-service bot merged commit c2f9f27 into google:master Jul 14, 2025
4 of 5 checks passed
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.

4 participants