Skip to content
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

server: Reuse the raw socket #129

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

Natolumin
Copy link
Member

Creating, then destroying a raw socket for every packet is very heavy. We can open the socket and keep it when first opening the (udp) listening socket, so that we are able to reuse the raw socket for further packets

Instead of managing the packet socket with manual syscalls, we can use the https://github.com/mdlayher/raw library (which we already imported before anyway) which will give us portability for free.
This does however mean that we cannot support dynamically setting the interface differently for each packet (at the moment) as the library requires an interface when binding to the socket; so this requires binding to a specific interface at startup (rather than relying on out-of-band data when receiving packets to choose the outgoing interface).
For DHCP servers this is a very common behavior (and we even required it until not that long ago) so I'm happy to take that tradeoff until we can update the upstream library to support the other usecase if needed.

Finally, if we can't setup the raw socket, we can then fallback to sending over broadcast (which is allowed by the RFC), ensuring that the server can keep working without for example CAP_NET_RAW

@codecov
Copy link

codecov bot commented Apr 24, 2021

Codecov Report

Attention: Patch coverage is 3.26087% with 89 lines in your changes missing coverage. Please review.

Project coverage is 41.93%. Comparing base (28988eb) to head (c8c93e2).

Files Patch % Lines
server/sendEthernet_linux.go 0.00% 68 Missing ⚠️
server/handle.go 11.76% 15 Missing ⚠️
server/serve.go 14.28% 6 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #129      +/-   ##
==========================================
- Coverage   42.62%   41.93%   -0.69%     
==========================================
  Files          24       24              
  Lines        1342     1364      +22     
==========================================
  Hits          572      572              
- Misses        679      701      +22     
  Partials       91       91              
Flag Coverage Δ
integtests 22.84% <3.26%> (-0.63%) ⬇️
unittests 46.87% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@insomniacslk
Copy link
Member

For DHCP servers this is a very common behavior (and we even required it until not that long ago) so I'm happy to take that tradeoff until we can update the upstream library to support the other usecase if needed.

I am OK with this behaviour

@Natolumin
Copy link
Member Author

This version should also cover #131 now

Creating, then destroying a raw socket for every packet is very heavy.
We can open the socket and keep it when first opening the listening
socket, so that we are able to reuse it

This however requires knowing and binding to an interface at socket
creation time (which is fairly standard for dhcpv4) due to a limitation
of the library we use

Signed-off-by: Anatole Denis <anatole@unverle.fr>
When deferring to the system routing, the ip address is automatically
selected by the kernel.
We were previously using the message's ServerIPAddr, but that is
incorrect as a source address since that is the address of the next
server, and not necessarily the address of the current server or an
address that is routable by the client

Signed-off-by: Anatole Denis <anatole@unverle.fr>
Signed-off-by: Anatole Denis <anatole@unverle.fr>
With the deprecation of mdhlayer/raw and its replacement with
mdhlayer/packet, we lost BSD support for AF_PACKET sockets
Since the server code degrades gracefully if tryOpenRawSock fails,
compile a stub to let the server compile and start outside linux

Signed-off-by: Anatole Denis <anatole@unverle.fr>
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.

None yet

2 participants