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

remove InetAddr from streamer/src/recvmmsg.rs #558

Merged
merged 3 commits into from
Apr 5, 2024
Merged

remove InetAddr from streamer/src/recvmmsg.rs #558

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d0a85c9
remove InetAddr from streamer/src/recvmmsg.rs
yihau Apr 3, 2024
6f7c506
remove 'allow deprecated'
yihau Apr 4, 2024
0d04bf1
add ref link
yihau Apr 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 21 additions & 11 deletions streamer/src/recvmmsg.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,5 @@
//! The `recvmmsg` module provides recvmmsg() API implementation

#[cfg(target_os = "linux")]
#[allow(deprecated)]
use nix::sys::socket::InetAddr;
pub use solana_perf::packet::NUM_RCVMMSGS;
use {
crate::packet::{Meta, Packet},
Expand All @@ -12,7 +9,11 @@ use {
use {
itertools::izip,
libc::{iovec, mmsghdr, sockaddr_storage, socklen_t, AF_INET, AF_INET6, MSG_WAITFORONE},
std::{mem, os::unix::io::AsRawFd},
std::{
mem,
net::{Ipv4Addr, Ipv6Addr, SocketAddr, SocketAddrV4, SocketAddrV6},
os::unix::io::AsRawFd,
},
};

#[cfg(not(target_os = "linux"))]
Expand Down Expand Up @@ -43,22 +44,31 @@ pub fn recv_mmsg(socket: &UdpSocket, packets: &mut [Packet]) -> io::Result</*num
}

#[cfg(target_os = "linux")]
#[allow(deprecated)]
fn cast_socket_addr(addr: &sockaddr_storage, hdr: &mmsghdr) -> Option<InetAddr> {
fn cast_socket_addr(addr: &sockaddr_storage, hdr: &mmsghdr) -> Option<SocketAddr> {
use libc::{sa_family_t, sockaddr_in, sockaddr_in6};
const SOCKADDR_IN_SIZE: usize = std::mem::size_of::<sockaddr_in>();
const SOCKADDR_IN6_SIZE: usize = std::mem::size_of::<sockaddr_in6>();
if addr.ss_family == AF_INET as sa_family_t
&& hdr.msg_hdr.msg_namelen == SOCKADDR_IN_SIZE as socklen_t
{
let addr = addr as *const _ as *const sockaddr_in;
return Some(unsafe { InetAddr::V4(*addr) });
// ref: https://github.com/rust-lang/socket2/blob/65085d9dff270e588c0fbdd7217ec0b392b05ef2/src/sockaddr.rs#L167-L172
let addr = unsafe { &*(addr as *const _ as *const sockaddr_in) };
return Some(SocketAddr::V4(SocketAddrV4::new(
Ipv4Addr::from(addr.sin_addr.s_addr.to_ne_bytes()),
u16::from_be(addr.sin_port),
)));
Comment on lines +56 to +59
Copy link

Choose a reason for hiding this comment

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

I don't know if these conversions are correct, and it is odd that one uses to_ne_bytes and the other from_be.
Is there any reference for where these come from?
Or can we add some test coverage here?

Similarly for Ipv6Addr below.

Copy link
Member Author

Choose a reason for hiding this comment

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

I literally copy from socket2: https://github.com/rust-lang/socket2/blob/65085d9dff270e588c0fbdd7217ec0b392b05ef2/src/sockaddr.rs#L165-L193

Copy link

Choose a reason for hiding this comment

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

Can you please put that link as a comment in the code?

I also see they have a different code for #[cfg(windows)]. any reason to not include that here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Can you please put that link as a comment in the code?

sure! fixed: 0d04bf1

I also see they have a different code for #[cfg(windows)]. any reason to not include that here?

afaik cast_socket_addr has #[cfg(target_os = "linux")] so it will never touch the windows condition
https://github.com/anza-xyz/agave/pull/558/files#diff-2d88fdbf6278e2d0250658ffc2903c9a6701e9b68e3e2303a23691b50bc298e2R46-R47

}
if addr.ss_family == AF_INET6 as sa_family_t
&& hdr.msg_hdr.msg_namelen == SOCKADDR_IN6_SIZE as socklen_t
{
let addr = addr as *const _ as *const sockaddr_in6;
return Some(unsafe { InetAddr::V6(*addr) });
// ref: https://github.com/rust-lang/socket2/blob/65085d9dff270e588c0fbdd7217ec0b392b05ef2/src/sockaddr.rs#L174-L189
let addr = unsafe { &*(addr as *const _ as *const sockaddr_in6) };
return Some(SocketAddr::V6(SocketAddrV6::new(
Ipv6Addr::from(addr.sin6_addr.s6_addr),
u16::from_be(addr.sin6_port),
addr.sin6_flowinfo,
addr.sin6_scope_id,
)));
}
error!(
"recvmmsg unexpected ss_family:{} msg_namelen:{}",
Expand Down Expand Up @@ -118,7 +128,7 @@ pub fn recv_mmsg(sock: &UdpSocket, packets: &mut [Packet]) -> io::Result</*num p
for (addr, hdr, pkt) in izip!(addrs, hdrs, packets.iter_mut()).take(nrecv) {
pkt.meta_mut().size = hdr.msg_len as usize;
if let Some(addr) = cast_socket_addr(&addr, &hdr) {
pkt.meta_mut().set_socket_addr(&addr.to_std());
pkt.meta_mut().set_socket_addr(&addr);
}
}
Ok(nrecv)
Expand Down
Loading