Skip to content

Don't call getsockname on every packet#11850

Merged
maskit merged 6 commits intoapache:masterfrom
maskit:getsockname
Nov 20, 2024
Merged

Don't call getsockname on every packet#11850
maskit merged 6 commits intoapache:masterfrom
maskit:getsockname

Conversation

@maskit
Copy link
Member

@maskit maskit commented Nov 8, 2024

UnixUDPConnection should have a local copy of the local address so no need to bother Kernel.

@maskit maskit added the UDP label Nov 8, 2024
@maskit maskit self-assigned this Nov 8, 2024
@maskit maskit requested a review from JosiahWI November 19, 2024 20:34
@JosiahWI
Copy link
Contributor

@maskit What's the reason for changing the socket storage type from sockaddr_in6 to sockaddr_storage?

@maskit
Copy link
Member Author

maskit commented Nov 20, 2024

Because it seemed more appropriate type, although sockkaddr_in6 is technically large enough for our use case.

Copy link
Contributor

@JosiahWI JosiahWI left a comment

Choose a reason for hiding this comment

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

Thanks. It's nice not to waste resources calling getsockname when the DNSConnection already has a local copy. It looks like that local copy should always exist since it's created in UDPBind.

I'm sad to see the introduction of reinterpret_casts, but it seems sockaddr_storage is designed for this kind of purpose, and the casts are guaranteed by POSIX in most cases not to violate strict aliasing, which is a nice bonus even though we don't enforce strict aliasing requirements.

@maskit
Copy link
Member Author

maskit commented Nov 20, 2024

It looks like that local copy should always exist since it's created in UDPBind.

This may not be true on outgoing QUIC connections, although we currently don't support it.

@maskit maskit merged commit a8932cf into apache:master Nov 20, 2024
@JosiahWI
Copy link
Contributor

I think it's set on the QUIC connections as well.

@cmcfarlen cmcfarlen added this to the 10.0.3 milestone Dec 4, 2024
@cmcfarlen
Copy link
Contributor

Cherry-picked to v10.0.x

cmcfarlen pushed a commit that referenced this pull request Dec 4, 2024
* Don't call getsockname on every packet

* fix errors

* fix errors

* fix errors

* fix errors

* fix wording

(cherry picked from commit a8932cf)
masaori335 pushed a commit to masaori335/trafficserver that referenced this pull request May 29, 2025
* Don't call getsockname on every packet

* fix errors

* fix errors

* fix errors

* fix errors

* fix wording

(cherry picked from commit a8932cf)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: Picked-10.0.3

Development

Successfully merging this pull request may close these issues.

3 participants