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.

Sign up for GitHub

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

Jump to bottom

Add support for IP_RECVERR #1514

Merged
merged 1 commit into from
Sep 7, 2021
Merged

Add support for IP_RECVERR #1514

merged 1 commit into from
Sep 7, 2021

Conversation

cemeyer
Copy link
Contributor

@cemeyer cemeyer commented Sep 6, 2021

This pull request adds support for setting the IP_RECVERR (and corresponding IPV6_RECVERR) sockopt, and then decoding the resulting control messages from recvmsg(). It is a Linux-specific API.

This PR extends #1161 by:

  • Making the address associated with a IpvXRecvErr control message optional (recvmsg documentation claims it is not provided for some kinds of error), per @WGH-.
  • Adding basic tests for both IPv4 and IPv6 (blat a UDP packet at a hopefully-unoccupied port on localhost and observe ICMP port unreachable).
  • Adding a Changelog entry.

I added some trivial doc comments on the ControlMessageOwned variants, but I'm not sure exactly what documentation you had in mind earlier on #1161.

@cemeyer
Copy link
Contributor Author

cemeyer commented Sep 6, 2021 •
edited
Loading

This failure is fun. What we've discovered is that something about libc::ECONNREFUSED is wrong on Linux-MIPS64 (BE and EL). (It's 111 on most platforms, but 146 in the MIPS ABI.) The libc value is the MIPS ABI, but we're getting 111 from the kernel...

(This might be a QEMU emulation bug for MIPS64 targets.)

failures:

---- sys::test_socket::linux_errqueue::test_recverr_v4 stdout ----
thread 'main' panicked at 'assertion failed: `(left == right)`
  left: `111`,
 right: `146`', test/sys/test_socket.rs:1931:9

@cemeyer
Copy link
Contributor Author

cemeyer commented Sep 6, 2021 •
edited
Loading

Several QEMU targets are hitting bogus address families in the control message associated "error origin:" Linux arm gnueabi, Linux armv7 gnueabihf, Linux MIPS, Linux mipsel. Probably have to just disable QEMU tests as broken for now.

@WGH-
Copy link

WGH- commented Sep 6, 2021

Is it qemu-system or qemu-user? If it's the latter, I'm not particularly surprised.

@cemeyer
Copy link
Contributor Author

cemeyer commented Sep 6, 2021 •
edited
Loading

Is it qemu-system or qemu-user? If it's the latter, I'm not particularly surprised.

My guess is user, although I'm not sure how to tell from the cirrus config.

So, I think both QEMU emulation bugs relate to do_sendrecvmsg_locked() (QEMU user): https://github.com/qemu/qemu/blob/master/linux-user/syscall.c#L3282-L3289

Both problems appears to be bugs in host_to_target_cmsg():

  • sa_family for sockaddrs associated with IP_RECVERR control messages is being butchered on some platforms (looks like maybe 32- / 64-bit issue).
  • ee_errno is not being correctly translated from host to target on platforms where sa_family was fine.

The sa_family one is tricky -- we do appear to do host_to_target_sockaddr translation. Perhaps that isn't fully functional? Alternatively, if the control message does not contain an offender address, or if it does not exactly match the size we expect, we just skip straight to 'unimplemented' and do a memcpy instead of translating. Different alignment requirements between 32/64-bit might cause that, although I'm not seeing it immediately.

ee_errno bug is pretty obvious here (and same for ipv6). Edit: filed: https://gitlab.com/qemu-project/qemu/-/issues/602 .

@asomers
Copy link
Member

asomers commented Sep 6, 2021

Oof, that's a complicated test. And the QEMU bug doesn't suprise me; there are plenty more where it came from. This PR LGTM. Just squash your commits and I'll merge it.

@cemeyer
Add support for IP_RECVERR and IPV6_RECVERR
d23e7c7 
Setting these options enables receiving errors, such as ICMP errors from
the network, via `recvmsg()` with `MSG_ERRQUEUE`.

Adds new `Ipv{4,6}RecvErr` variants to `ControlMessageOwned`.  These
control messages are produced when `Ipv4RecvErr` or `Ipv6RecvErr`
options are enabled on a raw or datagram socket.

New tests for the functionality can be run with `cargo test --test test
test_recverr`.

This commit builds on an earlier draft of the functionality authored by
Matthew McPherrin <git@mcpherrin.ca>.
@cemeyer
Copy link
Contributor Author

cemeyer commented Sep 7, 2021

Thanks! Squashed.

asomers
asomers approved these changes Sep 7, 2021
View reviewed changes
Copy link
Member

@asomers asomers left a comment

Choose a reason for hiding this comment

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

bors r+

@bors
Copy link
Contributor

bors bot commented Sep 7, 2021

Build succeeded:

@bors bors bot merged commit 2142ec3 into nix-rust:master Sep 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@asomers asomers asomers approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@cemeyer @WGH- @asomers