search

mdlayher / raw

Package raw enables reading and writing data at the device driver level for a network interface. MIT Licensed.
MIT License
425 stars 71 forks source link

Set read timeout on file descriptor #13

Closed corny closed 7 years ago

corny commented 7 years ago

Using poll before read is simple and works with timeouts.

mdlayher commented 7 years ago

This looks promising! Will do a review tomorrow. Excited to get some proper timeouts in here. I've been meaning to do it for Linux for ages.

corny commented 7 years ago

It doesn't seem to work properly yet. I'm still debugging.

corny commented 7 years ago

I did int(duration.Nanoseconds()/1000) instead of int(duration.Nanoseconds()/1000/1000). The timeout works now.

I'm not sure which errors to return best. Since poll also exists in Linux this can (better) generic solution instead of reading from non-blocking sockets periodically.

I have just written a simple DHCPv4 client that uses your library: https://github.com/digineo/go-dhclient

corny commented 7 years ago

I figured out how to set read timeouts on file descriptors. This might be the best solution. Please check out my changes and do some tests. Tests still have to be fixed.

corny commented 7 years ago

The recvfrom syscall does not return immediately on close of the socket. It only returns after the timeout - tested on Linux and FreeBSD. We should find a solution for this. Same problem with poll. There is not interruption on closing of the socket.

See also: closing a socket won't wake up recv()

This C example works: https://stackoverflow.com/a/8050401/3368559 The shutdown syscall does not work with raw sockets. It returns operation not supported

My suggestion: We should use multiple recvfrom (Linux) or read (BSD) syscalls with short timeouts (100-1000 ms?) until the deadline is reached.

mdlayher commented 7 years ago

That approach seems fine to me, so long as we can hide the complexity under our implementation of net.PacketConn.

mdlayher commented 7 years ago

Thanks for taking this on, by the way. I really appreciate it.

mdlayher commented 7 years ago

Fixes #4.

corny commented 7 years ago

Tests should be fine now. Please review and squash+merge if you don't request any changes.

corny commented 7 years ago

I removed the time.Duration type. VS code shows the variable as int value. But it does not change anything.

mdlayher commented 7 years ago

Thanks again. Will merge once the Mac build passes.

corny commented 7 years ago

Finally done ... after 42 minutes :sleeping:

mdlayher commented 7 years ago

Yeah, the Travis Mac runner is pretty awful sometimes.

corny commented 7 years ago

Thanks a lot!