Skip to content

Conversation

@WeidiDeng
Copy link
Collaborator

No description provided.

@vnxme
Copy link
Collaborator

vnxme commented Aug 27, 2025

@WeidiDeng, could you please comment on the purpose of these changes? Do they allow for logging UDP connections properly? Was this PR intended to be merged after #305?

@WeidiDeng
Copy link
Collaborator Author

The pr title is self explanatory. Logging udp connections is already properly done, because local address isn't logged anywhere.

This pr is out of date with the master. Whatever relationships it had don't matter anymore since there are conflicts needs to be solved.

@vnxme
Copy link
Collaborator

vnxme commented Aug 27, 2025

This pr is out of date with the master. Whatever relationships it had don't matter anymore since there are conflicts needs to be solved.

I can see the conflicts, and they are resolvable I suppose, if we do need these changes.

The pr title is self explanatory. Logging udp connections is already properly done, because local address isn't logged anywhere.

No doubt you have the best understanding of this PR as its author. I can read it "extracts local address for UDP sockets", but why? What does it enhance or fix? What do/will we use these extracted local addresses for?

Sorry, if my questions seem dumb. My only intention is actually to understand how this PR makes caddy-l4 better and why it hasn't been merged for over three months while having no comments and/or objections.

@WeidiDeng
Copy link
Collaborator Author

For udp sockets, by default, operating system only provides remote address for every packet read, not local address, i.e., the address in which the packet comes from. To acquire this address, special option needs to be turned on for the socket, and we will use another method to read the packet and its associated address data. This address data has a format that's platform dependent and will be parsed as the platform requires.

It enhances the proxy protocol for udp, which for now forwards the dummy address 127.0.0.1 as the local address if not available because stdlib will return the address we pass to ListenPacket and most of the time it's 0.0.0.0 and invalid for proxy protocol.

I guess no one cares the local address as long as the proxy protocol works for their application.

@vnxme
Copy link
Collaborator

vnxme commented Aug 27, 2025

It enhances the proxy protocol for udp, which for now forwards the dummy address 127.0.0.1 as the local address if not available because stdlib will return the address we pass to ListenPacket and most of the time it's 0.0.0.0 and invalid for proxy protocol.

Thank you, that's what I missed.

I guess no one cares the local address as long as the proxy protocol works for their application.

It would be a nice feature to have correct local addresses for both TCP and UDP though. Don't know why you are so sceptical about this PR given it's you who suggest these changes, the code has already been written and it presumably works fine.

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