search

rust-lang / socket2

Advanced configuration options for sockets.
https://docs.rs/socket2
Apache License 2.0
683 stars 227 forks source link

Add MSG_CONFIRM and MSG_DONTROUTE to RecvFlags #499

Closed pd0wm closed 8 months ago

pd0wm commented 8 months ago

This is useful when reading from SocketCAN sockets. The MSG_CONFIRM flag is used to indicate a successful transmission of a CAN frame.

Reference: https://www.kernel.org/doc/html/next/networking/can.html#raw-socket-option-can-raw-join-filters

Is there any reason the raw flags value is not exposed to the user? There might be more drivers that are using non-standard flags to communicate a certain state.

Thomasdezeeuw commented 8 months ago

Is there any reason the raw flags value is not exposed to the user? There might be more drivers that are using non-standard flags to communicate a certain state.

We're trying to provide a cross platform API, but for Windows RecvFlags is completely fake.

pd0wm commented 8 months ago

Looks like the build is failing due to missing fields for some targets. Shall I limit the new functions to target_os = "linux"?

Thomasdezeeuw commented 8 months ago

Looks like the build is failing due to missing fields for some targets. Shall I limit the new functions to target_os = "linux"?

It's it's Linux only then yes. But it will require the all feature as well.

pd0wm commented 8 months ago

Should be supported on Android as well. Is dc71380335fde833a3b8d66616e806c17a060891 what you mean by requiring both the all feature and gating on the os?

pd0wm commented 8 months ago

I think MSG_CONFIRM is normally used as a flag for calling sendmsg, and not used on received messages. So not sure if I can trigger this with an end-to-end test without using SocketCAN. SocketCAN cannot be easily tested inside a docker container, as the host needs to create a virtual SocketCAN device first.

Should I add the flags to the impl std::fmt::Debug as well? https://github.com/rust-lang/socket2/blob/master/src/sys/unix.rs#L579-L589

I can add a test that the value is false under normal circumstances here: https://github.com/rust-lang/socket2/blob/master/tests/socket.rs#L685-L686

Thomasdezeeuw commented 8 months ago

Should I add the flags to the impl std::fmt::Debug as well? https://github.com/rust-lang/socket2/blob/master/src/sys/unix.rs#L579-L589

I can add a test that the value is false under normal circumstances here: https://github.com/rust-lang/socket2/blob/master/tests/socket.rs#L685-L686

Yes to both. After that we can merge.

pd0wm commented 8 months ago

Should I add the flags to the impl std::fmt::Debug as well? https://github.com/rust-lang/socket2/blob/master/src/sys/unix.rs#L579-L589 I can add a test that the value is false under normal circumstances here: https://github.com/rust-lang/socket2/blob/master/tests/socket.rs#L685-L686

Yes to both. After that we can merge.

Already done :)

Thomasdezeeuw commented 8 months ago

Thanks @pd0wm

pd0wm commented 8 months ago

Thanks for your quick feedback!