recvmmsg is strange

[pacak]

recvmmsg takes data which is almost but not quite a mutable array reference to prepared buffers and returns a vector of RecvMsg<'a> which contains a reference to data, at least for as long as lifetimes are concerned. At this moment information about received packets is contained in two places: whatever is used to create data - payload we got and a vector that recvmmsg returns. To do anything with a packet we need both - payload from data and size of payload from the vector, but as long as lifetimes are concerned we can't do anything to data until we release the reference to the vector.

Even the test case seem to cheat here by size of the data sent rather than data received and changing it to do something similar to this

         for (buf, r) in msgs.iter().zip(res.iter()) {
                ....
          } 

results in

cannot borrow `msgs` as immutable because it is also borrowed as mutable

And it seems like the only way to actually use it is to extract anything from the resulting vector first (by doing allocations and not holding any references to it), drop it and only then process the actual payload - more allocations, bigger and slower code which you are probably trying to avoid by using recvmmsg in the first place.

So questions:

1. Is there anything that prevents recvmmsg from accepting an array (or a slice) and producing an iterator with references to this slice to avoid allocations?
2. Should produced iterator (or vector) also contain references to payload buffers?

[asomers]

I don't 100% understand. In what way does "test case seem to cheat"? And I'm also not sure what you're asking for. Are you asking to eliminate the Vec::collect() call at the end of recvmmsg?

[pacak]

Words are hard, sorry about that :)

So test defines data to send at line 422, performs the recvmmsg call at 482

nix/test/sys/test_socket.rs

Line 442 in 515e99b

const DATA: [u8; 2] = [1,2];

and then checks the results in two separate actions:

        for RecvMsg { address, bytes, .. } in res.into_iter() {
            assert_eq!(AddressFamily::Inet, address.unwrap().family());
            assert_eq!(DATA.len(), bytes);
        }

        for buf in &receive_buffers {
            assert_eq!(&buf[..DATA.len()], DATA);
        }

res here contains information about the bytes received and holds a mutable reference to buf, buf contains the data received itself so you can't access both at once - try combining those checks into a single iterator as you would do if you want to consume the data.

Cheating: Note that the second check uses DATA.len() - something that's only available on the sender side (or inside the first iterator - but you have to make a copy).

So two changes:

• to eliminate all the allocations inside recvmmsg, I think it should be possible to work with just preallocated buffers plus what's on a stack.
• to provide an immutable (or mutable) reference to buf from inside the res (in terms of test's variables)

Also I'm not asking you to make the changes, just to confirm if my understanding is correct - I'll make a patch myself.

[n47h4n13l]

I can refer to this problem. When using recvmmsg to obtain kernel timestamps I also stumbled over this.

The problem is the use of a single lifetime to bind the whole RecvMmsgData struct to the output of the recvmmsg method when it actually only uses the reference to the cmsg_buffer field. Thus, the introduction of a second lifetime in RecvMmsgData (and unfortunately some PhantomData) and two additional lifetimes in the recvmmsg method - one for the reference to the struct and one for the struct itself - can eliminate the problem you have. Then it is possible to zip the meta data received and the preallocated buffers to handle them simultaneously without a need to clone the meta data.

pub struct RecvMmsgData<'a, 'b, I>
    where I: AsRef<[IoVec<&'b mut [u8]>]> + 'b,
{
    pub iov: I,
    pub cmsg_buffer: Option<&'a mut Vec<u8>>,
    pub phantom: PhantomData<&'b ()>,
}

pub fn recvmmsg<'c, 'a: 'c, 'b: 'c, I>(
    fn: RawFd,
    data: impl std::iter::IntoIterator<Item=&'c mut RecvMmsgData<'a, 'b, I>,
        IntoIter=impl ExactSizeIterator + Iterator<Item=&'c mut RecvMmsgData<'a, 'b, I>>>,
    flags: MsgFlags,
    timeout: Option<crate::sys::time::TimeSpec>
) -> Result<Vec<RecvMsg<'a>>>
    where I: AsRef<[IoVec<&'b mut [u8]>]> + 'b,
{
    [...]
}

This will do, but is a breaking change.
Of course, if it is possible to get rid of the allocations inside recvmmsg, go ahead, that would be nice...

[hongshengjie]

The sendmmsg and recvmmsg API is really hard to use

[asomers]

Amen. recvmmsg is one of those things that makes me wish I'd become a day laborer instead of a programmer. Well, almost. But @n47h4n13l 's proposal sounds ok. Have you actually implemented it yet? Would you be willing to make a PR?