raw: use runtime epoll support

[hugelgupf]

This allows us to call Close() while an existing Read/Write call is in progress.

Yeah, this is ugly af.

[hugelgupf]

Hah, wow, I didn't see #38.

[hugelgupf]

This should be a more complete version of #38 now -- sorry, I didn't see that PR before. Going down this path because of a cleanup I'm doing of DHCP clients. Would appreciate seeing it merged soon.

[hugelgupf]

Thanks for the quick review! Updated.

[hugelgupf]

Thanks!

[stapelberg]

This change makes the raw package require Go 1.12. Was that intentional? If no, perhaps we should keep the old code around and use build constraints to support older Go versions again.

[mdlayher]

TBH my preference is to require 1.12 because it provides a much more efficient mechanism for timeouts, but when @acln0 made a comparable change for my netlink package, compatibility shims were also added.

If there are concerns about compatibility with prior versions of Go, I am happy to accept a patch for that. But my policy thus far has been that folks should expect to use the latest stable Go for my packages, and vendor if they're on an older release of Go.

I understand this may not be an ideal policy, but ultimately, we're all volunteers and we do the best that we can.

[mdlayher]

I'm happy to note this in the readme if that would be useful.

[stapelberg]

Documenting the policy in the README would certainly be a good idea :)

[mdlayher]

Will do!

[mdlayher]

Noted in https://github.com/mdlayher/raw/commit/764d452d77af302c17e326fad58f3dd0b5d3cf06. Thanks for prompting me to do so.

[stapelberg]

This commit broke router7’s dhcp4 client: https://github.com/rtr7/router7/issues/25

After a git revert f434146a17ecdf185e28b9ae8457ef45ee82077a, everything works as expected. At HEAD, dhcp4 gets one reply, then fails to ever send packets on the network interface, resulting in the DHCP lease expiring.

Haven’t investigated it any further, but perhaps this is sufficient to spot the mistake?

[mdlayher]

A patch of a very similar structure was applied to my netlink package and does not appear to have any adverse effects when dealing with numerous send/receive calls.

@hugelgupf do you see this in your application?

I was thinking last night that it'd probably be a good idea to get a better integration test harness set up around this package; perhaps setting up a veth pair and using them to push Ethernet frames back and forth could help.

[hugelgupf]

I'll take a look today, but I actually wrote this patch to improve the dhcpv4 client in insomniacslk/dhcp, which rtr7 happens to use I think. (PR is not yet merged.)

[stapelberg]

I'll take a look today, but I actually wrote this patch to improve the dhcpv4 client in insomniacslk/dhcp, which rtr7 happens to use I think. (PR is not yet merged.)

router7 used to use github.com/d2g/dhcp4 until commit https://github.com/rtr7/router7/commit/8c55c5ba444cb079c254604787da78dd6e0d26d2.

For DHCPv6, router7 uses github.com/insomniacslk/dhcp/dhcpv6.

[hugelgupf]

@stapelberg do you have some way of reproducing this in a QEMU environment?

I swapped out u-root's dhclient for your dhcp4 to use with my QEMU test env + dhcp4 test server there, and things work fine:

~/> dhcp4
2019/03/04 21:20:58 lease: {RenewAfter:2019-03-04 21:25:58.414139074 +0000 GMT m=+300.054814319 ClientIP:192.168.1.0 SubnetMask: Router: DNS:[]}

Not sure where to go from here.

[stapelberg]

The initial release isn’t the problem, the renewal is. Do renewals work for you?

[mdlayher]

I'm going to open a new issue so this is easier to track.

[mdlayher]

See #42.

[stapelberg]

FYI, as another data point: upon requiring Go 1.12, google/gopacket dropped the dependency on raw: https://github.com/google/gopacket/commit/1bbb32a78d0d8a2f2dcd6ec7f6ce0c37b57eab3d

In general, I think supporting the current stable and last stable Go version is a decent policy which should give people enough time to migrate. Up to you in the end, just wanted to let you know that people care.

[mdlayher]

Unfortunate, but based on the commit message, it appears to be a justified change for more than just that reason (zero allocations, ancillary data).