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.

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

Switch sendmmsg wrapper to use stack allocated libc types #3569

Open
steviez opened this issue Nov 11, 2024 · 1 comment
Open

Switch sendmmsg wrapper to use stack allocated libc types #3569

steviez opened this issue Nov 11, 2024 · 1 comment
Assignees

Comments

@steviez
Copy link

steviez commented Nov 11, 2024

Problem

On Linux based validators (ie all validators on MNB as far as we know), we use sendmmsg to potentially send multiple packets while incurring a single system call. In order to use sendmmsg, we have to populate some libc types; we do this in batch_send():

#[cfg(target_os = "linux")]
pub fn batch_send<S, T>(sock: &UdpSocket, packets: &[(T, S)]) -> Result<(), SendPktsError>
where
S: Borrow<SocketAddr>,
T: AsRef<[u8]>,
{
let size = packets.len();
let mut iovs = vec![MaybeUninit::uninit(); size];
let mut addrs = vec![MaybeUninit::zeroed(); size];
let mut hdrs = vec![MaybeUninit::uninit(); size];
for ((pkt, dest), hdr, iov, addr) in izip!(packets, &mut hdrs, &mut iovs, &mut addrs) {
mmsghdr_for_packet(pkt.as_ref(), dest.borrow(), iov, addr, hdr);
}
// mmsghdr_for_packet() performs initialization so we can safely transmute
// the Vecs to their initialized counterparts
let _iovs = unsafe { mem::transmute::<Vec<MaybeUninit<iovec>>, Vec<iovec>>(iovs) };
let _addrs = unsafe {
mem::transmute::<Vec<MaybeUninit<sockaddr_storage>>, Vec<sockaddr_storage>>(addrs)
};
let mut hdrs = unsafe { mem::transmute::<Vec<MaybeUninit<mmsghdr>>, Vec<mmsghdr>>(hdrs) };
sendmmsg_retry(sock, &mut hdrs)
}

Note that we allocating vectors here:

let mut iovs = vec![MaybeUninit::uninit(); size];
let mut addrs = vec![MaybeUninit::zeroed(); size];
let mut hdrs = vec![MaybeUninit::uninit(); size];

Proposed Solution

sendmmsg has a limit of 1024 packets per call*, so we could allocate 1024-element fixed size arrays on the stack to avoid the extra allocations every time this function is called. The three types in question have following size (as reported by std::mem::size_of):

  • libc::iovec ==> 16 bytes
  • libc::mmsghdr ==> 64 bytes
  • libc::sockaddr_storage ==> 128 bytes

So, in total that would be:

(16 + 64 + 128) * 1024 = 212,992 = 208 kB

This seems like a reasonable tradeoff given how often this function is getting called

*https://man7.org/linux/man-pages/man2/sendmmsg.2.html

@steviez
Copy link
Author

steviez commented Nov 11, 2024

Something similarly related but maybe best for another issue - Gossip and Repair both spawn a solRspondr thread

pub fn responder(
name: &'static str,
sock: Arc<UdpSocket>,
r: PacketBatchReceiver,
socket_addr_space: SocketAddrSpace,
stats_reporter_sender: Option<Sender<Box<dyn FnOnce() + Send>>>,
) -> JoinHandle<()> {

which calls batch_send() via recv_send():
fn recv_send(
sock: &UdpSocket,
r: &PacketBatchReceiver,
socket_addr_space: &SocketAddrSpace,
stats: &mut Option<StreamerSendStats>,
) -> Result<()> {
let timer = Duration::new(1, 0);
let packet_batch = r.recv_timeout(timer)?;
if let Some(stats) = stats {
packet_batch.iter().for_each(|p| stats.record(p));
}
let packets = packet_batch.iter().filter_map(|pkt| {
let addr = pkt.meta().socket_addr();
let data = pkt.data(..)?;
socket_addr_space.check(&addr).then_some((data, addr))
});
batch_send(sock, &packets.collect::<Vec<_>>())?;
Ok(())
}

In recv_send(), we pull a single PacketBatch from the receiver and pass to batch_send(). Once recv_timeout() yields one batch, it could be beneficial to call try_recv() until we get 1024 packets total.

And on a similar vein, we may benefit from patching some of the packets on the other side of the channel:

let mut pull_requests = vec![];
let mut pull_responses = vec![];
let mut push_messages = vec![];
let mut prune_messages = vec![];
let mut ping_messages = vec![];
let mut pong_messages = vec![];

I haven't dug through this code extensively, but I see that we send to the solRspondr thread in multiple places. Of course, waiting to process all messages could introduce some latency (vs. pushing out the packets in smaller batches); I'm not an expert on the gossip side so we'd have to pull someone else in to weigh in on this aspect

@cpubot cpubot self-assigned this Nov 13, 2024
@steviez steviez changed the title Switch sendmmsg wrapper to use stack allocated Switch sendmmsg wrapper to use stack allocated libc types Nov 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants